>>>>> "TM" == Trond Myklebust <trond.myklebust@fys.uio.no> writes:
TM> I've already corrected it for the NFS write-gathering code and
TM> smbfs in the 'ac' series since it gives a downright bug in the
TM> NFS code (you copy onto a page before checking whether it is
TM> being flushed or not).
> flushed in what sense? If you mean writing a page out to the
> NFS server, then it is a NFS bug. Because mmap can do that as
> well.
With NFS you do not have true permission checking. Thus you need some
form of synchronization between 2 different tasks writing to the same
inode. If they are writing to the same page, then you want to flush
the write of task 1 before copying the stuff from task 2 in order to
prevent task 2 from riding on the credentials of task 1. "This is not
bug, but a feature" 8-) (see below).
TM> 'generic_file_write' should not do anything other than the
TM> minimum required to call the lower level routines.
TM> NB: It should NOT set PG_lock, or PG_uptodate. Nor should it
TM> copy the data. All these things should be done by the lower
TM> level.
> There is a strong argument for copying the data.
> 1) permissions checks should not be done in updatepage. (which
> doesn't appear to be the problem)
It is: The user's credentials want to be checked by the server. The
client does not have all the necessary information to make absolutely
sure that the user is authorized to write. This is specially true of
NFSv2 which has no 'ACCESS' call for checking permissions upon opening
the file. Permission checks are only really made during the actual
write. (Yes, this is a 'bug/feature' of NFSv2).
Imagine user A is allowed to write: he fills a page. User B who is not
allowed to write by the server ('cos /etc/group is desynchronized
between client/server or something like that) modifies the page before
it is flushed asynchronously. Since user A set up the write request,
his credentials are used, and the write is accepted. Security
problem...
> 2) You can mmap the file for writing.
> In which case the write is also done for you. Nothing
> should depend on the data in the page cache being immutable,
> during a write.
> And because the mmap case can copy new data onto the page,
> there is no reason we should save ourselves a little complexity
> in the individual updatepage routines as well. Of course if
> updatepage fails PG_error should be set.
In the write gathering code, PG_lock is set while the page is actually
being written (during the RPC call). That should avoid the above
problem.
You're probably right about PG_updatepage. I don't think that would be
a problem, but IMHO, copy_from_user in 'generic_file_write' is
definitely the wrong thing to do...
Cheers,
Trond
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/