+ /* Is the file is open for append? Sorry, we don't do that. */ + if (file->f_flags & O_APPEND) { + goto Einval; + } + /* File needs write support. * The inode's perms must say it's ok, * and we must have a store method. @@ -312,6 +302,10 @@ static int check_perm(struct inode * ino file->private_data = buffer; } else error = -ENOMEM; + + /* Set mode bits to disallow seeking. */ + file->f_mode &= ~(FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE); + goto Done;
Einval: @@ -368,7 +343,7 @@ static int sysfs_release(struct inode * struct file_operations sysfs_file_operations = { .read = sysfs_read_file, .write = sysfs_write_file, - .llseek = generic_file_llseek, + .llseek = no_llseek, .open = sysfs_open_file, .release = sysfs_release, }; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Fri, Jan 21, 2005 at 02:49:39PM -0800, Mitch Williams wrote: > This patch causes sysfs to return errors if the caller attempts to append > to or seek on a sysfs file.
And what happens to it today if you do either of these?
> On Fri, Jan 21, 2005 at 02:49:39PM -0800, Mitch Williams wrote: > > This patch causes sysfs to return errors if the caller attempts to > append > > to or seek on a sysfs file.
> And what happens to it today if you do either of these?
> Also, isn't this two different things?
Appending and seeking are obviously two different operations, but the result is the same to the sysfs file system. Because the store method doesn't have an offset argument, it must assume that all writes are based from the beginning of the buffer.
So if your sysfs file contains "123" and you do echo "45" >> mysysfsfile instead of the expected "12345", you end up with "45" in the file with no errors. Opening the file, seeking, and writing gives the same type of behavior, with no errors.
This patch just sets a few flags to make sure that errors are returned when this behavior is seen. Logically then, the two "features" do the same thing (set flags), and prevent the same behavior (writing wrong contents without error). Hence, I grouped them into one patch.
However, if you want two even simpler patches, I'm willing to comply. Of the three patches I sent, this is the most important to me.
> Please, no {} for one line if statements. Like the one above it :)
I'll be glad to fix this and resubmit. I prefer to not have braces either, but I've seen a bunch of places in the kernel where people do it, so I really wasn't sure which was right. It's not really called out in the coding style doc either. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Mon, Jan 24, 2005 at 10:16:37AM -0800, Mitch Williams wrote:
> On Sat, 22 Jan 2005, Greg KH wrote:
> > On Fri, Jan 21, 2005 at 02:49:39PM -0800, Mitch Williams wrote: > > > This patch causes sysfs to return errors if the caller attempts to > > append > > > to or seek on a sysfs file.
> > And what happens to it today if you do either of these?
> > Also, isn't this two different things?
> Appending and seeking are obviously two different operations, but the > result is the same to the sysfs file system. Because the store method > doesn't have an offset argument, it must assume that all writes are based > from the beginning of the buffer.
> So if your sysfs file contains "123" and you do > echo "45" >> mysysfsfile > instead of the expected "12345", you end up with "45" in the file with no > errors. Opening the file, seeking, and writing gives the same type of > behavior, with no errors.
Ick, yeah, but users shouldn't be doing that :)
Anyway, ok, I'll accept this kind of patch, to give errors for that.
> However, if you want two even simpler patches, I'm willing to comply. Of > the three patches I sent, this is the most important to me.
Yes, could you split it up?
> > Please, no {} for one line if statements. Like the one above it :)
> I'll be glad to fix this and resubmit. I prefer to not have braces > either, but I've seen a bunch of places in the kernel where people do it, > so I really wasn't sure which was right. It's not really called out in > the coding style doc either.
