Hi,
Bryan Steele wrote on Fri, Jul 19, 2019 at 06:14:56PM -0400:
> On Sat, Jul 20, 2019 at 12:03:03AM +0200, Jesper Wallin wrote:
>> On Fri, Jul 19, 2019 at 05:14:03PM -0400, Bryan Steele wrote:
>>> I suspect that in secure/-S mode, the :pre[serve] should either be
>>> disabled, or modified to stop calling sendmail. The mail it is sending
>>> is purely advisory, and should be easy to disable. See common/recover.c.
>> Oh, you're right. A bit ironic that I didn't notice the exec violation
>> due to the fork being permitted now. Thanks for pointing this out!
>> Scrap my old patch, here's a better proposal:
> ok brynet@
No, that patch is not OK either.
It breaks :pre in -S mode because rcv_mailfile() is a blantant misnomer
that actually has two purposes:
1. Create the mail file to indicate that there is something recoverable
2. and then actually send the mail.
While step 2 must be skipped, step 1 for creating the recover.* file
is still needed, or "vi -r" will later complain "vi: No files to recover"
even though a vi.* file exists in /tmp/vi.recover/. Yes, i freely
admit that the design is horribly contorted.
So here is a better patch avoiding that problem.
It is also safer because it is easier to see that fork(2)
can no longer be reached. Otherwise, you would need to understand
that even though rcv_init() calls rcv_mailfile() and rcv_mailfile()
calls rcv_email(), fork(2) cannot be reached in that way because
isync is 0 in rcv_mailfile().
Yours,
Ingo
P.S.
See below the patch for my analysis of the code, which you may or may
not find helpful while verifying the patch.
Index: common/recover.c
===================================================================
RCS file: /cvs/src/usr.bin/vi/common/recover.c,v
retrieving revision 1.29
diff -u -p -r1.29 recover.c
--- common/recover.c 10 Nov 2017 18:25:48 -0000 1.29
+++ common/recover.c 21 Jul 2019 15:45:59 -0000
@@ -821,6 +821,15 @@ rcv_email(SCR *sp, int fd)
struct stat sb;
pid_t pid;
+ /*
+ * In secure mode, our pledge(2) includes neither "proc"
+ * nor "exec". So simply skip sending the mail.
+ * Later vi -r still works because rcv_mailfile()
+ * already did all the necessary setup.
+ */
+ if (O_ISSET(sp, O_SECURE))
+ return;
+
if (_PATH_SENDMAIL[0] != '/' || stat(_PATH_SENDMAIL, &sb) == -1)
msgq_str(sp, M_SYSERR,
_PATH_SENDMAIL, "not sending email: %s");
There are three call paths to rcv_mailfile() and rcv_email():
* rcv_init() -> rcv_mailfile()
purpose: set up .rcv_fd and .rcv_mpath
for locking purposes and in case of later dying from a signal
calls to rcv_init() are always safe because isync == 0,
i.e. rcv_email() is never called
* rcv_sync() -> rcv_email() directly
purpose: emergency saving while dying from SIGTERM;
RCV_EMAIL is not set in any other situation
so the direct call to rcv_email() must be neutered
note: RCV_SNAPSHOT is NOT set in this case
* rcv_sync() -> rcv_mailfile() -> rcv_email()
purpose: manual "pre" command
RCV_SNAPSHOT is not set in any other situation
note: RCV_EMAIL is NOT set in this case
here, the rcv_email() inside rcv_mailfile() must be neutered
