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--;
        }
 

Reply via email to