Re: fs: Uninitialized memory read at take_dentry_name_snapshot

[Al Viro <viro@ZenIV.linux.org.uk>]

On Mon, Sep 04, 2017 at 11:11:04PM +0900, Tetsuo Handa wrote:
> Al Viro wrote:
> > On Mon, Sep 04, 2017 at 09:12:38PM +0900, Tetsuo Handa wrote:
> > > Hello.
> > > 
> > > I hit kmemcheck splat on commit 49d31c2f389acfe8 ("dentry name snapshots") using linux-next-20170901.
> > > Changing to strncpy() fixes this problem, but using strncpy() only if CONFIG_KMEMCHECK=y is better
> > > for performance?
> > 
> > *shrug*
> > 
> > If anything, use dentry->d_name.len + 1 instead of DNAME_INLINE_LEN there, but
> > that's really a false positive.
> 
> A false positive? Isn't it because we do
> 
>   dentry = kmem_cache_alloc(dentry_cache, GFP_KERNEL);
>   (...snipped...)
>   dname = dentry->d_iname;
>   (...snipped...)
>   memcpy(dname, name->name, name->len);
>   dname[name->len] = 0;
> 
> at __d_alloc() which leaves a room for some of d_iname[] bytes uninitialized?
> So, I think either pad explicitly at __f_alloc() or use dentry->d_name.len + 1 is needed.

It's the same situation as with
	struct foo {char s[10];int v;} *p, *q;
	p = malloc(sizeof struct foo);
	strcpy(p->s, "foo");
	p->v = 69;
	...
	q = malloc(sizeof struct foo);
	memcpy(q, p, sizeof(struct foo));

Sure, bytes after NUL are uninitialized, but that NUL is there
in both copies.  FWIW, kmemcheck could simply copy the information
about source to that about destination - in the example above
we'd end up with "q->v initialized, along with the first 4 bytes of
q->s", which matches the reality.  BTW, how do you handle struct
assignments?