On Wed, 22 Feb 2023 17:11:47 +0100, Moritz Buhl wrote:
> The tool finds a path to ex_search where fsb.f_fsid is uninitialized.
>
> ex_search compares the potentially uninitialized stack data:
>
> ex_search(fsid_t *fsid)
> {
> struct exportlist *ep;
>
> ep = exphead;
> while (ep) {
> if (ep->ex_fs.val[0] == fsid->val[0] &&
> ...
>
> Is it sufficient to zero fsb?
> Is this really reachable?
I believe that it is. However, I don't know why we wouldn't just
want to skip ex_search, etc when bad is set. Also, there is no
reason to use a critical section now that the SIGHUP handle just
sets a flag. Something like this (untested), should be fine.
It needs testing by someone who actually uses NFS though...
- todd
Index: sbin/mountd/mountd.c
===================================================================
RCS file: /cvs/src/sbin/mountd/mountd.c,v
retrieving revision 1.89
diff -u -p -u -r1.89 mountd.c
--- sbin/mountd/mountd.c 6 Aug 2020 17:57:32 -0000 1.89
+++ sbin/mountd/mountd.c 22 Feb 2023 17:17:44 -0000
@@ -736,7 +736,6 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
char rpcpath[RPCMNT_PATHLEN+1], dirpath[PATH_MAX];
struct hostent *hp = NULL;
struct exportlist *ep;
- sigset_t sighup_mask;
int defset, hostset;
struct fhreturn fhr;
struct dirlist *dp;
@@ -746,8 +745,6 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
u_short sport;
long bad = 0;
- sigemptyset(&sighup_mask);
- sigaddset(&sighup_mask, SIGHUP);
saddr = transp->xp_raddr.sin_addr.s_addr;
sport = ntohs(transp->xp_raddr.sin_port);
switch (rqstp->rq_proc) {
@@ -790,9 +787,10 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
fprintf(stderr, "stat failed on %s\n", dirpath);
bad = ENOENT; /* We will send error reply later */
}
+ if (bad)
+ goto mount_error;
/* Check in the exports list */
- sigprocmask(SIG_BLOCK, &sighup_mask, NULL);
ep = ex_search(&fsb.f_fsid);
hostset = defset = 0;
if (ep && (chk_host(ep->ex_defdir, saddr, &defset, &hostset) ||
@@ -800,13 +798,6 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
chk_host(dp, saddr, &defset, &hostset)) ||
(defset && scan_tree(ep->ex_defdir, saddr) == 0 &&
scan_tree(ep->ex_dirl, saddr) == 0))) {
- if (bad) {
- if (!svc_sendreply(transp, xdr_long,
- (caddr_t)&bad))
- syslog(LOG_ERR, "Can't send reply");
- sigprocmask(SIG_UNBLOCK, &sighup_mask, NULL);
- return;
- }
if (hostset & DP_HOSTSET)
fhr.fhr_flag = hostset;
else
@@ -817,11 +808,7 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
if (imsg_getfh(dirpath, (fhandle_t *)&fhr.fhr_fh) < 0) {
bad = errno;
syslog(LOG_ERR, "Can't get fh for %s", dirpath);
- if (!svc_sendreply(transp, xdr_long,
- (caddr_t)&bad))
- syslog(LOG_ERR, "Can't send reply");
- sigprocmask(SIG_UNBLOCK, &sighup_mask, NULL);
- return;
+ goto mount_error;
}
if (!svc_sendreply(transp, xdr_fhs, (caddr_t)&fhr))
syslog(LOG_ERR, "Can't send reply");
@@ -842,9 +829,11 @@ mntsrv(struct svc_req *rqstp, SVCXPRT *t
} else
bad = EACCES;
- if (bad && !svc_sendreply(transp, xdr_long, (caddr_t)&bad))
- syslog(LOG_ERR, "Can't send reply");
- sigprocmask(SIG_UNBLOCK, &sighup_mask, NULL);
+ if (bad) {
+ mount_error:
+ if (!svc_sendreply(transp, xdr_long, (caddr_t)&bad))
+ syslog(LOG_ERR, "Can't send reply");
+ }
return;
case RPCMNT_DUMP:
if (!svc_sendreply(transp, xdr_mlist, NULL))
@@ -958,11 +947,7 @@ xdr_explist(XDR *xdrsp, caddr_t cp)
{
struct exportlist *ep;
int false = 0, putdef;
- sigset_t sighup_mask;
- sigemptyset(&sighup_mask);
- sigaddset(&sighup_mask, SIGHUP);
- sigprocmask(SIG_BLOCK, &sighup_mask, NULL);
ep = exphead;
while (ep) {
putdef = 0;
@@ -973,12 +958,9 @@ xdr_explist(XDR *xdrsp, caddr_t cp)
goto errout;
ep = ep->ex_next;
}
- sigprocmask(SIG_UNBLOCK, &sighup_mask, NULL);
- if (!xdr_bool(xdrsp, &false))
- return (0);
- return (1);
+ if (xdr_bool(xdrsp, &false))
+ return (1);
errout:
- sigprocmask(SIG_UNBLOCK, &sighup_mask, NULL);
return (0);
}
@@ -1050,7 +1032,6 @@ void
new_exportlist(int signo)
{
gothup = 1;
-
}
/*
