lkml.org 
[lkml]   [2013]   [Dec]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] fs: bits in .close_on_exec are only defined for matching bits in .open_fds bits
Date
Flag close-on-exec can only be set on an allocated (but perhaps not yet
installed) file descriptor. So if the bit in struct fdtable .open_fds array
is not set, then value of matching bit in the .close_on_exec array is
meaningless.

This patch rely on this property to
- remove initialization of unused part of .close_on_exec array;
- remove clear of .close_on_exec bit when releasing a file descriptor.
The patch takes care of adding the required check on .open_fds bit
before looking for .close_on_exec bit.

Link: http://lkml.kernel.org/r/1386796107-4197-1-git-send-email-ydroneaud@opteya.com
Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
---

Hi Al and Mateusz,

First of all, thank you for reviewing my previous patch and pointing out
the error I've missed.

Please consider this new patch which take the opposite approach:
my previous patch assumed that .close_on_exec bit where defaulting to 0,
but you prove this was a wrong assumption. This new patch assume that
.close_on_exec bit are in a unknown, meaningless value when the file
descriptor is not allocated. This way, there's no need to clear the value
when releasing a file descriptor, and there's no need to initialize the
.close_on_exec array.

Unlike my previous patch, I haven't yet tested it. It's known to compile.

Please try to find some corner cases I've missed in this other attempt.

Regards.


fs/file.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/fs/file.c b/fs/file.c
index 4a78f981557a..3016e09d0290 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -78,7 +78,7 @@ static void copy_fdtable(struct fdtable *nfdt, struct fdtable *ofdt)
memcpy(nfdt->open_fds, ofdt->open_fds, cpy);
memset((char *)(nfdt->open_fds) + cpy, 0, set);
memcpy(nfdt->close_on_exec, ofdt->close_on_exec, cpy);
- memset((char *)(nfdt->close_on_exec) + cpy, 0, set);
+ /* remaining portion of close_on_exec left uninitialized */
}

static struct fdtable * alloc_fdtable(unsigned int nr)
@@ -335,7 +335,7 @@ struct files_struct *dup_fd(struct files_struct *oldf, int *errorp)
int start = open_files / BITS_PER_LONG;

memset(&new_fdt->open_fds[start], 0, left);
- memset(&new_fdt->close_on_exec[start], 0, left);
+ /* remaining portion of close_on_exec left uninitialized */
}

rcu_assign_pointer(newf->fdt, new_fdt);
@@ -599,7 +599,6 @@ int __close_fd(struct files_struct *files, unsigned fd)
if (!file)
goto out_unlock;
rcu_assign_pointer(fdt->fd[fd], NULL);
- __clear_close_on_exec(fd, fdt);
__put_unused_fd(files, fd);
spin_unlock(&files->file_lock);
return filp_close(file, files);
@@ -622,10 +621,9 @@ void do_close_on_exec(struct files_struct *files)
fdt = files_fdtable(files);
if (fd >= fdt->max_fds)
break;
- set = fdt->close_on_exec[i];
+ set = fdt->close_on_exec[i] & fdt->open_fds[i];
if (!set)
continue;
- fdt->close_on_exec[i] = 0;
for ( ; set ; fd++, set >>= 1) {
struct file *file;
if (!(set & 1))
@@ -772,7 +770,7 @@ bool get_close_on_exec(unsigned int fd)
bool res;
rcu_read_lock();
fdt = files_fdtable(files);
- res = close_on_exec(fd, fdt);
+ res = fd_is_open(fd, fdt) && close_on_exec(fd, fdt);
rcu_read_unlock();
return res;
}
--
1.8.4.2


\
 
 \ /
  Last update: 2013-12-12 13:41    [W:0.086 / U:0.268 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site