Re: debugfs & vfs file permission issue?

[Robin Getz]

On Tue 6 Jan 2009 01:32, Mike Frysinger pondered:
> On Tue, Jan 6, 2009 at 01:19, Greg KH wrote:
> > On Tue, Jan 06, 2009 at 12:54:58AM -0500, Mike Frysinger wrote:
> >> On Tue, Jan 6, 2009 at 00:48, Greg KH wrote:
> >> > On Mon, Jan 05, 2009 at 09:57:07PM -0500, Robin Getz wrote:
> >> >> On 2.6.28-rc2, If I create a debugfs file with
> >> >>
> >> >>     debugfs_create_x16("SPORT1_TX", 0200 , parent, 0xFFC00910);
> >> >
> >> > Um, are you are passing in a pointer to a known memory location
> >> > properly?  Why would it be ok for the kernel to directly read that
> >> > location?
> >>
> >> it's a nommu system and the 0xffc00000+ addresses are always available
> >
> > Writable but not readable?  Isn't hardware fun :)
> 
> yeah and we have some readable but not writable addresses too :/
> 
> >> >> Although the file shows up as write only (no read):
> >> >>
> >> >> root:/> ls -l /sys/kernel/debug/blackfin/SPORT/SPORT1_TX
> >> >> --w-------    1 root     root            0 Jan  1
> >> >> 2007 /sys/kernel/debug/blackfin/SPORT/SPORT1_TX
> >> >>
> >> >> root:/> cat /sys/kernel/debug/blackfin/SPORT/SPORT1_TX
> >> >>
> >> >> Still works - and causes the read to occur, which crashes :(
> >> >
> >> > You're root, you can do anything :)
> >>
> >> any thoughts on how to declare debugfs files that are read or write
> >> only ?  we'll have to add new helper functions or have it be a
> >> parameter or declare our own debugfs file ?
> >
> > Just use debugfs_create_file() and use your own read/write functions to
> > prevent a read or write from happening no matter what.  No new debugfs
> > infrastructure should be needed.
> 
> would it be useful for this to be a common function ?  probably not so
> much considering (i'm assuming) no one else has asked so far for such
> a beast ...

Today there is:

file.c:DEFINE_SIMPLE_ATTRIBUTE(fops_u8, debugfs_u8_get, debugfs_u8_set, "%llu\n");
file.c:DEFINE_SIMPLE_ATTRIBUTE(fops_u16, debugfs_u16_get, debugfs_u16_set, "%llu\n");
file.c:DEFINE_SIMPLE_ATTRIBUTE(fops_u32, debugfs_u32_get, debugfs_u32_set, "%llu\n");
file.c:DEFINE_SIMPLE_ATTRIBUTE(fops_u64, debugfs_u64_get, debugfs_u64_set, "%llu\n");
file.c:DEFINE_SIMPLE_ATTRIBUTE(fops_x8, debugfs_u8_get, debugfs_u8_set, "0x%02llx\n");
file.c:DEFINE_SIMPLE_ATTRIBUTE(fops_x16, debugfs_u16_get, debugfs_u16_set, "0x%04llx\n");
file.c:DEFINE_SIMPLE_ATTRIBUTE(fops_x32, debugfs_u32_get, debugfs_u32_set, "0x%08llx\n");

adding a readonly, and writeonly, and ensuring that when you call 
debugfs_create_*, the mode is checked, and the "correct" fops are set 
doesn't seem like it would be a bad idea? This would enforce the
kernel programmer's view on the world, and not allow pesky root users
to override things....

Greg - would you take something like that?

Or do you just want us to it as you suggested previously? and don't use the 
existing debugfs_create_* functions directly on wonky hardware registers?

Thanks
-Robin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/