linux-kernel - Re: [PATCH v2 3/3] intel_th: msu: Fix an off-by-one in attribute store

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] [day] [month] [year] [list]

Message-ID: <20181219192151.GA4413@kroah.com>
Date:   Wed, 19 Dec 2018 20:21:51 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Alexander Shishkin <alexander.shishkin@...ux.intel.com>
Cc:     Mathieu Poirier <mathieu.poirier@...aro.org>,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v2 3/3] intel_th: msu: Fix an off-by-one in attribute
 store

On Wed, Dec 19, 2018 at 05:19:22PM +0200, Alexander Shishkin wrote:
> The 'nr_pages' attribute of the 'msc' subdevices parses a comma-separated
> list of window sizes, passed from userspace. However, there is a bug in
> the string parsing logic wherein it doesn't exclude the comma character
> from the range of characters as it consumes them. This leads to an
> out-of-bounds access given a sufficiently long list. For example:
> 
> > # echo 8,8,8,8 > /sys/bus/intel_th/devices/0-msc0/nr_pages
> > ==================================================================
> > BUG: KASAN: slab-out-of-bounds in memchr+0x1e/0x40
> > Read of size 1 at addr ffff8803ffcebcd1 by task sh/825
> >
> > CPU: 3 PID: 825 Comm: npktest.sh Tainted: G        W         4.20.0-rc1+
> > Call Trace:
> >  dump_stack+0x7c/0xc0
> >  print_address_description+0x6c/0x23c
> >  ? memchr+0x1e/0x40
> >  kasan_report.cold.5+0x241/0x308
> >  memchr+0x1e/0x40
> >  nr_pages_store+0x203/0xd00 [intel_th_msu]
> 
> Fix this by accounting for the comma character.

Ugh, this is one major reason sysfs files are "one value per file".

You should never have to "parse" a sysfs file for something like this.

It's a bit too late now, but please, never create such a sysfs file
again :(

greg k-h