linux-kernel - Re: [RFC 2/3] file: allow callers to free the old file descriptor after dup2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZAeBIcZvQsB2N7eq@ip-172-31-38-16.us-west-2.compute.internal>
Date:   Tue, 7 Mar 2023 18:23:29 +0000
From:   Alok Tiagi <aloktiagi@...il.com>
To:     David Laight <David.Laight@...lab.com>
Cc:     "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "hch@...radead.org" <hch@...radead.org>,
        Tycho Andersen <tycho@...ho.pizza>
Subject: Re: [RFC 2/3] file: allow callers to free the old file descriptor
 after dup2

On Thu, Mar 02, 2023 at 10:21:11PM +0000, David Laight wrote:
> From: aloktiagi
> > Sent: 02 March 2023 18:22
> > 
> > Allow callers of do_dup2 to free the old file descriptor in case they need to
> > make additional operations on it.
> 
> That doesn't read right at all.
> 
> Whether or not this is a good idea (or can be done differently)
> the interface is horrid.
> 
> >  	if (tofree)
> > -		filp_close(tofree, files);
> > +		*fdfile = tofree;
> 
> Why not:
> 
> 	if (fdfile) [
> 		*fdfile = tofree;
> 	} else {
> 		if (tofree)
> 			filp_close(tofree, files);
> 	}
> 
> Then existing code just passes NULL and the caller can't 'forget'
> to intitalise fdfile.
> 
> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
> 

thank you for the review, I'll fix it in v2