linux-hardening - Re: [PATCH v2] seq_file: Unconditionally use vmalloc for buffer

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: <YFCq2TeHAl1s8/TX@kroah.com>
Date:   Tue, 16 Mar 2021 13:55:53 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     Kees Cook <keescook@...omium.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Michal Hocko <mhocko@...e.com>,
        Alexey Dobriyan <adobriyan@...il.com>,
        Lee Duncan <lduncan@...e.com>, Chris Leech <cleech@...hat.com>,
        Adam Nichols <adam@...mm-co.com>, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2] seq_file: Unconditionally use vmalloc for buffer

On Tue, Mar 16, 2021 at 12:43:12PM +0000, Al Viro wrote:
> On Tue, Mar 16, 2021 at 08:24:50AM +0100, Greg Kroah-Hartman wrote:
> 
> > > Completely agreed. seq_get_buf() should be totally ripped out.
> > > Unfortunately, this is going to be a long road because of sysfs's ATTR
> > > stuff, there are something like 5000 callers, and the entire API was
> > > designed to avoid refactoring all those callers from
> > > sysfs_kf_seq_show().
> > 
> > What is wrong with the sysfs ATTR stuff?  That should make it so that we
> > do not have to change any caller for any specific change like this, why
> > can't sysfs or kernfs handle it automatically?
> 
> Hard to tell, since that would require _finding_ the sodding ->show()
> instances first.  Good luck with that, seeing that most of those appear
> to come from templates-done-with-cpp...

Sure, auditing all of this is a pain, but the numbers that take a string
are low if someone wants to do that and convert them all to sysfs_emit()
today.

> AFAICS, Kees wants to protect against ->show() instances stomping beyond
> the page size.  What I don't get is what do you get from using seq_file
> if you insist on doing raw access to the buffer rather than using
> seq_printf() and friends.  What's the point?

I don't understand as I didn't switch kernfs to this api at all anyway,
as it seems to have come from the original sysfs code moving to kernfs
way back in 2013 with the work that Tejun did.  So I can't remember any
of that...

thanks,

greg k-h