Hi Al Viro,
On Tue, Aug 20, 2013 at 11:21 AM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> On Tue, Aug 20, 2013 at 10:55:40AM +0530, Arun KS wrote:
>> >From 932a134abeac597f18c86c513704709ad154994b Mon Sep 17 00:00:00 2001
>> From: Arun KS <arun.ks@xxxxxxxxxxxx>
>> Date: Mon, 19 Aug 2013 12:06:33 +0530
>> Subject: seq_file: Fix overflow condition in seq_commit
>>
>> seq_path()/seq_commit() is treating a d_path() failure as an overflow
>> condition, but it isn't.
>>
>> seq_read handles overflow by reallocating more buffer. And this
>> continues in a loop utill we increase the size of seq buf beyond
>> KMALLOC_MAX_SIZE and hence a WARN_ON.
>>
>> [ 363.192565] ------------[ cut here ]------------
>> [ 363.199462] WARNING: at mm/slab_common.c:377 kmalloc_slab+0x34/0x9c()
>> [ 363.208557] Modules linked in:
>> [ 363.212219] CPU: 1 PID: 1742 Comm: Binder_2 Tainted: G W3.10.0+ #17
>> [ 363.222930] [<c00151c4>] (unwind_backtrace+0x0/0x11c)
>> from[<c0011a24>] (show_stack+0x10/0x14)
>> [ 363.235229] [<c0011a24>] (show_stack+0x10/0x14) from
>> [<c0059fb0>](warn_slowpath_common+0x4c/0x68)
>> [ 363.247253] [<c0059fb0>] (warn_slowpath_common+0x4c/0x68)
>> from[<c0059fe4>] (warn_slowpath_null+0x18/0x1c)
>> [ 363.259307] [<c0059fe4>] (warn_slowpath_null+0x18/0x1c)
>> from[<c00fa400>] (kmalloc_slab+0x34/0x9c)
>> [ 363.270812] [<c00fa400>] (kmalloc_slab+0x34/0x9c) from
>> [<c010ec20>](__kmalloc+0x14/0x1fc)
>> [ 363.281433] [<c010ec20>] (__kmalloc+0x14/0x1fc) from
>> [<c012ef70>](seq_read+0x24c/0x438)
>> [ 363.291992] [<c012ef70>] (seq_read+0x24c/0x438) from
>> [<c011389c>](vfs_read+0xac/0x124)
>> [ 363.302398] [<c011389c>] (vfs_read+0xac/0x124) from
>> [<c0113a38>](SyS_read+0x3c/0x60)
>> [ 363.312591] [<c0113a38>] (SyS_read+0x3c/0x60) from
>> [<c000e180>](ret_fast_syscall+0x0/0x48)
>> [ 363.323303] ---[ end trace 46c6467e2db7bcd4 ]---
>>
>> Pass -ENOBUFS to seq_commit to signal an overflow condition and a
>> negative value for all other errors.
>
> Excuse me, _what_ other errors? Please, explain what errors can
> be returned by d_path/__d_path/dentry_path. Normally all of those
> return ERR_PTR(-ENAMETOOLONG) if the pathname to be generated would
> not fit into a buffer. In which case the only sane behaviour is
> to use a bigger buffer.
d_path expects the pathname to be less than 64 bytes. If its more than
64, it returns -ENAMETOOLONG.
File:fs/dcache.c
char *dynamic_dname(struct dentry *dentry, char *buffer, int buflen,
const char *fmt, ...)
{
va_list args;
char temp[64];
int sz;
va_start(args, fmt);
sz = vsnprintf(temp, sizeof(temp), fmt, args) + 1;
va_end(args);
if (sz > sizeof(temp) || sz > buflen)
return ERR_PTR(-ENAMETOOLONG);
buffer += buflen - sz;
return memcpy(buffer, temp, sz);
}
The string name which give error is,
"dev/ashmem/CursorWindow:
/data/data/com.android.providers.settings/databases/settings.db"
when sz>sizeof(temp), dynamic_dname returns -ENAMETOOLONG.
In this instance, sz is coming as 100.
This always happens from shared mem, after shmem_dname function was
added by the following commit.
commit 3451538a114d738a6528b1da58e19e7d8964c647
Author: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Date: Thu Feb 14 22:38:02 2013 -0500
shmem_setup_file(): use d_alloc_pseudo() instead of d_alloc()
Note that provided ->d_dname() reproduces what we used to get for
those guys in e.g. /proc/self/maps; it might be a good idea to change
that to something less ugly, but for now let's keep the existing
user-visible behaviour
Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Thanks,
Arun
>
> Could you please post the reproducer for that trace of yours?
> I might be missing something obvious, of course, but AFAICS you
> are just papering over a bug somewhere else. If you really
> manage to get d_path() with output that wouldn't fit into 128Mb,
> you have a whole lot of unpleasant problems beyond a warning in
> seq_read(). And silent truncation of pathnames that don't happen
> to fit into what's left of the default buffer is simply wrong -
> 4095-byte pathnames are perfectly fine.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
