[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Cluster-devel] [RFC PATCH 5/5] gfs2: Add xreaddir file operation and supporting functions




On 29/07/14 23:25, Abhijith Das wrote:

----- Original Message -----
From: "Jonathan Corbet" <corbet lwn net>
To: "Abhi Das" <adas redhat com>
Cc: linux-kernel vger kernel org, linux-fsdevel vger kernel org, cluster-devel redhat com
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.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]