Re: fs: gpf in simple_setattr

[Jan Kara <jack@suse.cz>]

On Fri 07-03-14 21:14:21, Sasha Levin wrote:
> On 03/06/2014 11:02 AM, Sasha Levin wrote:
> >On 03/05/2014 07:45 AM, Jan Kara wrote:
> >>On Tue 04-03-14 19:00:32, Sasha Levin wrote:
> >>>On 03/03/2014 04:40 PM, Jan Kara wrote:
> >>>>On Sat 01-03-14 15:05:21, Sasha Levin wrote:
> >>>>>>ping again?
> >>>>>>
> >>>>>>I've been working on it, but don't see an obvious issue.
> >>>>>>
> >>>>>>It does look like an access to invalid memory easily doable from
> >>>>>>userspace, so it should probably get fixed soon...
> >>>>   Hum, can you maybe dump the name in dentry passed to simple_setattr()? Or
> >>>>maybe even the whole path using dentry_path() (but not sure if that will
> >>>>be workable on half-torn-down fs)? Maybe it will give us a hint at which
> >>>>filesystem to look...
> >>>
> >>>It's just garbage, this is why I'm having a hard time making any progress with
> >>>this bug.
> >>   OK, but that is strange because we hold a reference to the dentry so
> >>noone should free it. So dentry->d_name should be valid... Is the rest of
> >>the dentry also garbage? E.g. does dentry->d_inode still point to the inode
> >>we call __mark_inode_dirty() on? Is dentry->d_sb == dentry->d_inode->i_sb?
> >>Also if the inode isn't completely garbage, we can maybe infer something
> >>from inode->i_op - that should point to some statically allocated
> >>operations struct so we should be able to guess fs type from that.
> >
> >It's actually pretty tricky. This issue being a race makes catching it at the right time
> >difficult.
> >
> >I've tried catching it in simple_setattr() before calling mark_inode_dirty() by testing
> >for the poison values inside inode, but they seem to be perfectly fine there and still
> >show up as bad within mark_inode_dirty().
> >
> >Then I tried trapping it inside mark_inode_dirty(), but at that point I usually get garbage
> >inside inode, and have no way to go back to dentry.
> >
> >Right now I'm just trying to dump everything that goes through simple_setattr() in hopes that
> >I could easily figure out what went wrong by looking at the log, but that just stops the bug
> >from reproducing.
> 
> I've tried the following code in simple_setattr() right before the call to mark_inode_dirty():
> 
>         p = dentry_path(dentry, pth, 200);
>         printk(KERN_ERR "doh: %s %s\n", p, inode->i_sb->s_type->name);
> 
> but it seems that while 'p' ends up being "/", inode->i_sb is garbage and we can't pull out anything
> about the file system.
  By garbage, do you mean that it is a poison, completely random data or does
inode->i_sb look like a valid pointer but just superblock isn't where it
points to?

> Any way I could get anything useful any other way?
  Hum, can you dump the whole contents of 'dentry' at that place? Maybe it
will tell us something.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR