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?
