On Wed, Oct 09, 2024 at 04:19:47PM +0200, Christian Brauner wrote:
> On Wed, Oct 09, 2024 at 11:50:36AM GMT, Steven Price wrote:
> > On 07/10/2024 18:43, Al Viro wrote:
> > > ->close_on_exec[] state is maintained only for opened descriptors;
> > > as the result, anything that marks a descriptor opened has to
> > > set its cloexec state explicitly.
> > >
> > > As the result, all calls of __set_open_fd() are followed by
> > > __set_close_on_exec(); might as well fold it into __set_open_fd()
> > > so that cloexec state is defined as soon as the descriptor is
> > > marked opened.
> > >
> > > Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> > > ---
> > > fs/file.c | 9 ++++-----
> > > 1 file changed, 4 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/fs/file.c b/fs/file.c
> > > index d8fccd4796a9..b63294ed85ec 100644
> > > --- a/fs/file.c
> > > +++ b/fs/file.c
> > > @@ -248,12 +248,13 @@ static inline void __set_close_on_exec(unsigned int fd, struct fdtable *fdt,
> > > }
> > > }
> > >
> > > -static inline void __set_open_fd(unsigned int fd, struct fdtable *fdt)
> > > +static inline void __set_open_fd(unsigned int fd, struct fdtable *fdt, bool set)
> > > {
> > > __set_bit(fd, fdt->open_fds);
> > > fd /= BITS_PER_LONG;
> >
> > Here fd is being modified...
> >
> > > if (!~fdt->open_fds[fd])
> > > __set_bit(fd, fdt->full_fds_bits);
> > > + __set_close_on_exec(fd, fdt, set);
> >
> > ... which means fd here isn't the same as the passed in value. So this
> > call to __set_close_on_exec affects a different fd to the expected one.
ACK.
> Good spot. Al, I folded the below fix so from my end you don't have to
> do anything unless you want to nitpick how to fix it. Local variable
> looked ugly to me.
Wait, folded it _where_? And how did it end up pushed to -next in the
first place?
<checks vfs/vfs.git>
FWIW, I do have a problem with your variant. My preferred incremental would be
simply to deal with close_on_exec bitmap between updating open_fds and full_fds_bits:
diff --git a/fs/file.c b/fs/file.c
index 70cd6d8c6257..7251d215048d 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -249,10 +249,10 @@ static inline void __set_close_on_exec(unsigned int fd, struct fdtable *fdt,
static inline void __set_open_fd(unsigned int fd, struct fdtable *fdt, bool set)
{
__set_bit(fd, fdt->open_fds);
+ __set_close_on_exec(fd, fdt, set);
fd /= BITS_PER_LONG;
if (!~fdt->open_fds[fd])
__set_bit(fd, fdt->full_fds_bits);
- __set_close_on_exec(fd, fdt, set);
}
static inline void __clear_open_fd(unsigned int fd, struct fdtable *fdt)
