linux-kernel - Re: [git pull] mnt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 17 Mar 2011 15:28:47 +0800
From:	Xiaotian Feng <xtfeng@...il.com>
To:	Al Viro <viro@...iv.linux.org.uk>, tony.luck@...el.com
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [git pull] mnt_devname queue

On Thu, Mar 17, 2011 at 3:23 PM, Al Viro <viro@...iv.linux.org.uk> wrote:
> On Thu, Mar 17, 2011 at 01:45:33PM +0800, Xiaotian Feng wrote:
>
>> I guess we need also switch pstore from ->get_sb() to ->mount()
>>
>> fs/pstore/inode.c:253: error: unknown field ???get_sb??? specified in initializer
>> fs/pstore/inode.c:253: warning: initialization makes integer from
>> pointer without a cast
>> fs/pstore/inode.c:253: error: initializer element is not computable at load time
>> fs/pstore/inode.c:253: error: (near initialization for
>> ???pstore_fs_type.fs_flags???)
>
> Yes.  I've just looked at that thing and I see several, er, issues.
>

Cc'ed author ...

> a) What the hell would you expect to happen if userland mounts it twice
> and unmount the first one?  pstore_sb = NULL, pstore_mnt = NULL, AFAICS.
>
> b) pstore_writefile() - struct file on stack?  Really?  Again, in the
> scenario above, what'll happen to you if pstore_mnt gets dropped and
> freed in the middle of all that?
>
> c) in the same function:
> +       memset(&f, '0', sizeof f);
> Ahem...
>
> d)
> +               if (pstore_is_mounted())
> +                       pstore_mkfile(PSTORE_TYPE_DMESG, psinfo->name, id,
> +                                     psinfo->buf, hsize + l1_cpy + l2_cpy,
> +                                     CURRENT_TIME, psinfo->erase);
>
> And what happens if it's unmounted between the check and use?  pstore_mkfile()
> pretty much starts with dereferencing pstore_sb...
>
> e) as the matter of fact, what happens if it's unmounted in the _middle_ of
> pstore_mkfile()?
>
> f) in general it's a lousy policy to tie that kind of difference in behaviour
> to "somebody has it mounted someplace at the moment", even without the races
> a-la (a)...
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/