On 29/07/14 23:25, Abhijith Das wrote:
----- Original Message -----
From: "Jonathan Corbet" <[email protected]>
To: "Abhi Das" <[email protected]>
Cc: [email protected], [email protected],
[email protected]
Sent: Tuesday, July 29, 2014 1:58:08 PM
Subject: Re: [RFC PATCH 5/5] gfs2: Add xreaddir file operation and supporting
functions
[snip]
+ if ((xc->xc_xattr_mask & XSTAT_XATTR_ALL) &&
+ lxd->xd_blob.xb_xattr_count) {
How can that be right? lxd is __user, it doesn't seem right to be
dereferencing it directly...?
Wouldn't the call to access_ok() at the start of the syscall take care of this?
All the
__user pointers point to areas within the user supplied buffer buf and overflow
past the
end of the buffer for the last lxd is checked for.
The 2/5 patch in this series adds the following in fs/readdir.c:
+SYSCALL_DEFINE5(xgetdents, unsigned int, fd, unsigned, flags, unsigned int,
mask,
+ void __user *, buf, unsigned int, count)
...
...
...
+ if (!access_ok(VERIFY_WRITE, buf, count))
+ return -EFAULT;
The access_ok() call only checks the source memory at the time of the
call, it is possible for the page to become invalid for reading or
writing to at any subsequent time. So that the kernel's accessor
functions are required here. I think sparse should warn about this too.
However, the intention was to show that the xreaddir approach is not
ideal due to its complexity, and also due to it being very much single
purpose compared with the readahead approach, and from that point of
view it shouldn't affect the performance measurements in this case,
Steve.
