linux-kernel - Re: [PATCH v1] seq_file: Fix overflow condition in seq

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]

Message-ID: <20130820055153.GH27005@ZenIV.linux.org.uk>
Date:	Tue, 20 Aug 2013 06:51:53 +0100
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Arun KS <arunks.linux@...il.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Matthew Wilcox <matthew@....cx>,
	Bruce Fields <bfields@...ldses.org>,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	vinayak menon <vinayakm.list@...il.com>,
	Nagachandra P <nagachandra@...il.com>,
	Vikram MP <mp.vikram@...il.com>
Subject: Re: [PATCH v1] seq_file: Fix overflow condition in seq_commit

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@...adcom.com>
> 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.

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-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/