Bug 9867 - sysfs files can be truncated
Summary: sysfs files can be truncated
Status: CLOSED CODE_FIX
Alias: None
Product: File System
Classification: Unclassified
Component: SysFS (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Greg Kroah-Hartman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-02-01 09:57 UTC by Ben Hutchings
Modified: 2008-06-02 00:30 UTC (History)
0 users

See Also:
Kernel Version: 2.6.24
Subsystem:
Regression: ---
Bisected commit-id:


Attachments

Description Ben Hutchings 2008-02-01 09:57:27 UTC
sysfs allows attribute files to be truncated, e.g. using ftruncate(), with the expected effect on their inode - but this doesn't change the "real" size of the file i.e. how much can be read from it.

The parameter validation for reading and writing binary attribute files is based the inode size and not the size specified in the file's bin_attribute, so it can be broken by this. For example, if we try using dd to write to such a file:

# pwd
/sys/bus/pci/devices/0000:08:00.0
# ls -l config
-rw-r--r--  1 root root 4096 Feb  1 17:35 config
# dd if=/dev/zero of=config bs=4 count=1
1+0 records in
1+0 records out
# ls -l config
-rw-r--r--  1 root root 0 Feb  1 17:50 config
# dd if=/dev/zero of=config bs=4 count=1 seek=128
dd: writing `config': No space left on device
1+0 records in
0+0 records out

Note that after the truncation to 0 by the first use of dd, parameter validation for read and write is disabled. Most bin_attribute read and write methods also validate the size and offset, but for some this will allow out-of-range access. (I don't see this as a security issue, because access to the sysfs files is privileged anyway. But the validation should remain for safety's sake!)

I think sysfs should ignore size changes or refuse them (by returning -EINVAL). The following change (modulo whitespace mangling) works for me:

--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -59,6 +59,8 @@
        if (error)
                return error;
 
+       iattr->ia_valid &= ~ATTR_SIZE; /* ignore size changes */
+
        error = inode_setattr(inode, iattr);
        if (error)
                return error;
--- END ---
Comment 1 Natalie Protasevich 2008-06-02 00:30:14 UTC
Patch is in, commit 40a2159abf3d0107bba359246554bd7d56f2171b - thanks.

Note You need to log in before you can comment on or make changes to this bug.