linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Kees Cook <keescook@chromium.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Michal Hocko <mhocko@suse.com>,
	Alexey Dobriyan <adobriyan@gmail.com>,
	Lee Duncan <lduncan@suse.com>, Chris Leech <cleech@redhat.com>,
	Adam Nichols <adam@grimm-co.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-hardening@vger.kernel.org
Subject: Re: [PATCH v2] seq_file: Unconditionally use vmalloc for buffer
Date: Tue, 16 Mar 2021 08:24:50 +0100	[thread overview]
Message-ID: <YFBdQmT64c+2uBRI@kroah.com> (raw)
In-Reply-To: <202103151336.78360DB34D@keescook>

On Mon, Mar 15, 2021 at 01:43:59PM -0700, Kees Cook wrote:
> On Mon, Mar 15, 2021 at 06:33:10PM +0000, Al Viro wrote:
> > On Mon, Mar 15, 2021 at 10:48:51AM -0700, Kees Cook wrote:
> > > The sysfs interface to seq_file continues to be rather fragile, as seen
> > > with some recent exploits[1]. Move the seq_file buffer to the vmap area
> > > (while retaining the accounting flag), since it has guard pages that
> > > will catch and stop linear overflows. This seems justified given that
> > > seq_file already uses kvmalloc(), is almost always using a PAGE_SIZE or
> > > larger allocation, has allocations are normally short lived, and is not
> > > normally on a performance critical path.
> > 
> > You are attacking the wrong part of it.  Is there any reason for having
> > seq_get_buf() public in the first place?
> 
> 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?

> However, since I also need to entirely rewrite the sysfs vs kobj APIs[1]
> for CFI, I'm working on a plan to fix it all at once, but based on my
> experience refactoring the timer struct, it's going to be a very painful
> and long road.

Oh yeah, that fun.  I don't think it's going to be as hard as you think,
as the underlying code is doing the "right thing" here, so this feels
like a problem in the CFI implementation more than anything else.

So what can I do today in sysfs to help fix the seq_get_buf() stuff?
What should it use instead?

thanks,

greg k-h

  reply	other threads:[~2021-03-16  7:25 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-15 17:48 Kees Cook
2021-03-15 18:33 ` Al Viro
2021-03-15 20:43   ` Kees Cook
2021-03-16  7:24     ` Greg Kroah-Hartman [this message]
2021-03-16 12:43       ` Al Viro
2021-03-16 12:55         ` Greg Kroah-Hartman
2021-03-16 13:01         ` Michal Hocko
2021-03-16 19:18         ` Kees Cook
2021-03-17 10:44           ` Greg Kroah-Hartman
2021-03-16  8:31 ` Michal Hocko
2021-03-16 19:08   ` Kees Cook
2021-03-17 12:08     ` Michal Hocko
2021-03-17 13:34       ` Greg Kroah-Hartman
2021-03-17 14:44         ` Michal Hocko
2021-03-17 14:56           ` Greg Kroah-Hartman
2021-03-17 15:20             ` Michal Hocko
2021-03-17 15:38               ` Greg Kroah-Hartman
2021-03-17 15:48                 ` Michal Hocko
2021-03-17 21:30                 ` Kees Cook
2021-03-18  8:07                   ` Greg Kroah-Hartman
2021-03-18 15:51                     ` Kees Cook
2021-03-18 17:56                       ` Greg Kroah-Hartman
2021-03-19 14:07 ` [seq_file] 5fd6060e50: stress-ng.eventfd.ops_per_sec -49.1% regression kernel test robot
2021-03-19 19:31   ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YFBdQmT64c+2uBRI@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=adam@grimm-co.com \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=cleech@redhat.com \
    --cc=keescook@chromium.org \
    --cc=lduncan@suse.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhocko@suse.com \
    --cc=viro@zeniv.linux.org.uk \
    --subject='Re: [PATCH v2] seq_file: Unconditionally use vmalloc for buffer' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).