linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Matteo Croce <mcroce@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Alexey Dobriyan <adobriyan@gmail.com>
Subject: Re: [PATCH 2/2] kernel: use sysctl shared variables for range check
Date: Wed, 10 Apr 2019 14:50:53 -0700	[thread overview]
Message-ID: <CAGXu5j+0sAc2fHTuYwYTLoHPDbnfxYuAEJ+tRfgc2Z5DDqp9Jg@mail.gmail.com> (raw)
In-Reply-To: <CAGnkfhwZEqX03H7PqsSTa=GVwZXX127hQO5YHzJasAEYtpLMZw@mail.gmail.com>

On Wed, Apr 10, 2019 at 12:24 PM Matteo Croce <mcroce@redhat.com> wrote:
>
> On Wed, Apr 10, 2019 at 8:46 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Mon, Apr 8, 2019 at 3:09 PM Matteo Croce <mcroce@redhat.com> wrote:
> > >
> > > Use the shared variables for range check, instead of declaring a local one
> > > in every source file.
> >
> > I was expecting this to be a tree-wide change for all the cases found
> > by patch 1's "git grep".
> >
>
> Hi Kees,
>
> I have already the whole patch ready, but I was frightened by the
> output of get_maintainer.pl, so I decided to split the patch into
> small pieces and send the first one.

Heh, sounds fine. Normally the big tree-wide changes go via Linus just
before cutting rc1 (or rc2). This is "only" 31 source files, though,
so maybe akpm wants to take these instead? Andrew, how do you feel
about that?

> Patches for /proc/sys/net and drivers/ are pretty big, and can be
> merged after the 1/2 inclusion.
>
> > Slight change to the grep for higher accuracy:
> >
> > $ git grep -E '\.extra[12].*&(zero|one|int_max)\b' |wc -l
> > 245
> >
>
> Right, my regexp wrongly matches also one_hundred, one_jiffy, etc.
> Anywqay, I did the changes by hand, so apart the commit message, the
> content should be safe.
>
> > Only 31 sources:
> > $ git grep -E '\.extra[12].*&(zero|one|int_max)\b' | cut -d: -f1 |
> > sort -u > /tmp/list.txt
> > $ wc -l /tmp/list.txt
> > 31
> >
> > One thing I wonder about is if any of these cases depend on the extra
> > variable being non-const (many of these are just "static int").
> >
> > $ egrep -H '\b(zero|one|int_max)\b.*=' $(cat /tmp/list.txt) | grep -v static
> >
> > Looks like none, so it'd be safe. How about doing this tree-wide for
> > all 31 cases? (Coccinelle might be able to help.)
>
> It could be true for other sysctl values like
> xpc_disengage_max_timelimit or fscache_op_wq, but it's very unlikely
> that someone writes, for example, 5 into a variable named "zero". If
> it does, it most likely a bug, so const is our friend.

I completely agree. :) I just wanted to make sure and it turned out
the grep was very easy.

-- 
Kees Cook

  reply	other threads:[~2019-04-10 21:51 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-08 22:09 [PATCH 0/2] sysctl: share commonly used constants Matteo Croce
2019-04-08 22:09 ` [PATCH 1/2] proc/sysctl: add shared variables for range check Matteo Croce
2019-04-10 22:18   ` Kees Cook
2019-04-08 22:09 ` [PATCH 2/2] kernel: use sysctl " Matteo Croce
2019-04-10 18:46   ` Kees Cook
2019-04-10 19:23     ` Matteo Croce
2019-04-10 21:50       ` Kees Cook [this message]
2019-04-10 22:30         ` Matteo Croce
2019-04-10 22:34           ` Kees Cook
2019-04-10 22:54             ` Matteo Croce
2019-04-10 22:59               ` Kees Cook
2019-04-16 23:45                 ` Andrew Morton
2019-04-17  3:21   ` Kees Cook
2019-04-17  3:22     ` 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=CAGXu5j+0sAc2fHTuYwYTLoHPDbnfxYuAEJ+tRfgc2Z5DDqp9Jg@mail.gmail.com \
    --to=keescook@chromium.org \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=mcroce@redhat.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).