linux-kernel - Re: [PATCH] cpumask: Omit terminating null byte in cpumap_print_{list,bitmask}_to

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: <20210917084526.uzpish2owb24szng@distanz.ch>
Date:   Fri, 17 Sep 2021 10:45:27 +0200
From:   Tobias Klauser <tklauser@...tanz.ch>
To:     Yury Norov <yury.norov@...il.com>
Cc:     "Song Bao Hua (Barry Song)" <song.bao.hua@...ilicon.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jonathan Cameron <jonathan.cameron@...wei.com>,
        "tiantao (H)" <tiantao6@...ilicon.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] cpumask: Omit terminating null byte in
 cpumap_print_{list,bitmask}_to_buf

On 2021-09-17 at 01:19:04 +0200, Yury Norov <yury.norov@...il.com> wrote:
> [CC Greg KH <gregkh@...uxfoundation.org>]
> 
> On Thu, Sep 16, 2021 at 10:53:39PM +0000, Song Bao Hua (Barry Song) wrote:
> > 
> > 
> > > -----Original Message-----
> > > From: Tobias Klauser [mailto:tklauser@...tanz.ch]
> > > Sent: Friday, September 17, 2021 10:27 AM
> > > To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>; Jonathan Cameron
> > > <jonathan.cameron@...wei.com>; tiantao (H) <tiantao6@...ilicon.com>; Song Bao
> > > Hua (Barry Song) <song.bao.hua@...ilicon.com>
> > > Cc: Andrew Morton <akpm@...ux-foundation.org>; Andy Shevchenko
> > > <andriy.shevchenko@...ux.intel.com>; Yury Norov <yury.norov@...il.com>; Peter
> > > Zijlstra <peterz@...radead.org>; linux-kernel@...r.kernel.org
> > > Subject: [PATCH] cpumask: Omit terminating null byte in
> > > cpumap_print_{list,bitmask}_to_buf
> > > 
> > > The changes in the patch series [1] introduced a terminating null byte
> > > when reading from cpulist or cpumap sysfs files, for example:
> > > 
> > >   $ xxd /sys/devices/system/node/node0/cpulist
> > >   00000000: 302d 310a 00                             0-1..
> > > 
> > > Before this change, the output looked as follows:
> > > 
> > >   $ xxd /sys/devices/system/node/node0/cpulist
> > >   00000000: 302d 310a                                0-1.
> > 
> > If we don't use xxd, I don't see any actual harm of this NULL byte
> > by cat, lscpu, numactl etc. this doesn't break them at all.
> 
> Barry, Tobias' script that uses xxd is userspace. Linux kernel never breaks
> userspace. 

FWIW, the example using xxd was just to illustrate the issue in a
concise way for the commit message. This is breaking other userspace
programs as well. Originally, I discovered this because Kubernetes'
kubelet was crashing on a bpf-next kernel. See [1] and following
comments for more information:

[1] https://github.com/cilium/cilium/pull/17394#issuecomment-920902042

> > if we only want to make sure the output is exactly same with before
> > for every single character, this patch is right.
> 
> We don't want to make the output exactly the same. The "0,1" would
> also work for the example above. But garbage characters following \0
> is a bug that should be fixed.

I think we also want to avoid the \0 itself, which is what this patch
does and is in line with previous behavior. It also looks like all other
sysfs files in that subtree expose the same content format (i.e. \n is
the last character, not \0).

Thanks,
Tobias