On 16/04/18(Mon) 09:09, Todd C. Miller wrote:
> On Mon, 16 Apr 2018 10:19:40 +0200, Martin Pieuchot wrote:
>
> > Diff below does FREF(9) earlier instead of incrementing `f_count' by hand.
> >
> > The error path is also updated to call FRELE(9) accordingly.
>
> Wouldn't it be less error prone to simply add:
>
> if (fp != NULL)
> FRELE(fp, p);
>
> to the fail label? If we get to fail, fp is either NULL or needs to
> drop a reference.
Sure, here's an updated diff. It also moves the FRELE(9) in the error
loop down as suggested by visa@.
Index: kern/uipc_usrreq.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
retrieving revision 1.123
diff -u -p -r1.123 uipc_usrreq.c
--- kern/uipc_usrreq.c 4 Jan 2018 10:45:30 -0000 1.123
+++ kern/uipc_usrreq.c 17 Apr 2018 07:48:09 -0000
@@ -838,6 +838,7 @@ morespace:
error = EBADF;
goto fail;
}
+ FREF(fp);
if (fp->f_count == LONG_MAX-2) {
error = EDEADLK;
goto fail;
@@ -845,7 +846,7 @@ morespace:
error = pledge_sendfd(p, fp);
if (error)
goto fail;
-
+
/* kqueue descriptors cannot be copied */
if (fp->f_type == DTYPE_KQUEUE) {
error = EINVAL;
@@ -854,7 +855,6 @@ morespace:
rp->fp = fp;
rp->flags = fdp->fd_ofileflags[fd] & UF_PLEDGED;
rp--;
- fp->f_count++;
if ((unp = fptounp(fp)) != NULL) {
unp->unp_file = fp;
unp->unp_msgcount++;
@@ -863,13 +863,15 @@ morespace:
}
return (0);
fail:
+ if (fp != NULL)
+ FRELE(fp, p);
/* Back out what we just did. */
for ( ; i > 0; i--) {
rp++;
fp = rp->fp;
- fp->f_count--;
if ((unp = fptounp(fp)) != NULL)
unp->unp_msgcount--;
+ FRELE(fp, p);
unp_rights--;
}
