linux-kernel - Re: [RFC] readdir mess

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] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 12 Aug 2008 14:04:25 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Al Viro <viro@...IV.linux.org.uk>
cc:	OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC] readdir mess

On Tue, 12 Aug 2008, Al Viro wrote:
> 
> 	Would you care to grep for vfs_readdir() in the tree?  It's not just
> sys_getdents(); for better of worse the thing had become a general-purpose
> iterator.  And I'm not suggesting to pass the damn thing to caller of
> sys_getdents().  At all.

Umm. What does that matter wrt what I said?

What does that matter wrt your bogus argument about EIO (which we do 
_wrong_ right now, exactly because we return EIO too eagerly, instead of 
returning the partial data we got)?

vfs_readdir() itself would not change AT ALL. The change I talked about 
was in the caller. Which you should realize, since I actually _called_ 
vfs_readdir() in that example code.

Here's a patch to get the _semantics_ I talked about, to make the thing 
more obvious.

But the actual _change_ I talked about would be to try to avoid using 
"buf.error" entirely, and just make all the (many, I know) filesystem 
readdir() functions return the callback value instead of 0. Which would 
mean that most of the users would be able to drop their "buf.error" use 
entirely, along with the ugly

	if (error >= 0)
		error = buf.error;

that I have in this patch.

(Side note: the "if (error >= 0)" approach is what old_readdir() already 
does, so that function actually gets the EIO case _correct_: it's only 
the newer sys_getdents() that is buggy)

			Linus

---
 fs/readdir.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/readdir.c b/fs/readdir.c
index 4e026e5..e4ded16 100644
--- a/fs/readdir.c
+++ b/fs/readdir.c
@@ -205,9 +205,8 @@ asmlinkage long sys_getdents(unsigned int fd, struct linux_dirent __user * diren
 	buf.error = 0;

 	error = vfs_readdir(file, filldir, &buf);
-	if (error < 0)
-		goto out_putf;
-	error = buf.error;
+	if (error >= 0)
+		error = buf.error;
 	lastdirent = buf.previous;
 	if (lastdirent) {
 		if (put_user(file->f_pos, &lastdirent->d_off))
@@ -216,7 +215,6 @@ asmlinkage long sys_getdents(unsigned int fd, struct linux_dirent __user * diren
 			error = count - buf.count;
 	}

-out_putf:
 	fput(file);
 out:
 	return error;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/