Messages in this thread | | | Date | Mon, 19 Apr 1999 14:32:00 +0200 (CEST) | Subject | Re: Q: generic_file_write sets PG_locked??? | From | Trond Myklebust <> |
| |
>>>>> " " == Eric W Biederman <ebiederm+eric@ccr.net> writes:
>>>>> "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/
| |