All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Manfred Spraul <manfred@colorfullife.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Davidlohr Bueso <dave@stgolabs.net>,
	Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Fabian Frederick <fabf@skynet.be>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ipc/sem: Avoid indexing past end of sem_array
Date: Mon, 15 May 2017 10:40:34 -0700	[thread overview]
Message-ID: <CAGXu5jJt9yB-4tbcBOBwqFB5STP7AVYFt-THpjHsCJNL7jdvKA@mail.gmail.com> (raw)
In-Reply-To: <e5e0fa5c-cc26-d05d-a2d3-3a10d191cc70@colorfullife.com>

On Sun, May 14, 2017 at 6:54 AM, Manfred Spraul
<manfred@colorfullife.com> wrote:
> Hi Kees,
>
> On 05/09/2017 12:23 AM, Kees Cook wrote:
>>
>> This changes the struct + trailing data pattern to using a void * so that
>> the end of sem_array is found without possibly indexing past the end which
>> can upset some static analyzers. Mostly, this ends up avoiding a cast
>> between different non-void types, which the future randstruct GCC plugin
>> was warning about.
>
> Two question:
> - Would the attached patch work with the randstruct plugin as well?
>   If we touch the code, then I would propose that we remove sem_base
> entirely.

I'll double check with your series, but I think your change makes
sense regardless (since it makes it very clear that there are
allocated sems after the struct due to the [0] entry).

>
> - ipc/util.h contains
>
>> #define ipc_rcu_to_struct(p)  ((void *)(p+1))
>
> Does this trigger a warning with randstruct as well?
> If we have to touch it, then I would remove it by merging struct
> kern_ipc_perm and struct ipc_rcu.
>
> And, obviously:
> Do you see any issues with the attached patch?

I'll test your series with the randstruct series and see what falls out. :)

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

  reply	other threads:[~2017-05-15 17:40 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-08 22:23 [PATCH] ipc/sem: Avoid indexing past end of sem_array Kees Cook
2017-05-14 13:54 ` Manfred Spraul
2017-05-15 17:40   ` Kees Cook [this message]
2017-05-15 17:19 ` [PATCH 0/2] Misc cleanups for ipc Manfred Spraul
2017-05-15 17:19   ` [PATCH 1/3] ipc/sem.c: remove sem_base, embed struct sem Manfred Spraul
2017-05-15 20:08     ` Andrew Morton
2017-05-15 22:16       ` Kees Cook
2017-05-16  5:46     ` Christoph Hellwig
2017-05-15 17:19   ` [PATCH 2/3] ipc: merge ipc_rcu and kern_ipc_perm Manfred Spraul
2017-05-16  0:03     ` Kees Cook
2017-05-19  5:52     ` [lkp-robot] [ipc] 2f93a15114: [No primary change] will-it-scale.time.involuntary_context_switches -99% kernel test robot
2017-05-15 17:19   ` [PATCH 3/3] include/linux/sem.h: Correctly document sem_ctime Manfred Spraul
2017-05-25 18:50 ` [PATCH 0/20 V3] Misc cleanups for ipc Manfred Spraul
2017-05-25 18:50   ` [PATCH 01/20] ipc/sem.c: remove sem_base, embed struct sem Manfred Spraul
2017-05-25 19:43     ` Kees Cook
2017-05-25 18:50   ` [PATCH 02/20] ipc: merge ipc_rcu and kern_ipc_perm Manfred Spraul
2017-05-25 19:34     ` Kees Cook
2017-05-26  3:37       ` Kees Cook
2017-05-25 18:50   ` [PATCH 03/20] include/linux/sem.h: Correctly document sem_ctime Manfred Spraul
2017-05-25 18:50   ` [PATCH 04/20] ipc: Drop non-RCU allocation Manfred Spraul
2017-05-25 19:35     ` Kees Cook
2017-05-25 18:50   ` [PATCH 05/20] ipc/sem: Do not use ipc_rcu_free() Manfred Spraul
2017-05-25 18:50   ` [PATCH 06/20] ipc/shm: " Manfred Spraul
2017-05-25 18:50   ` [PATCH 07/20] ipc/msg: " Manfred Spraul
2017-05-25 18:50   ` [PATCH 08/20] ipc/util: Drop ipc_rcu_free() Manfred Spraul
2017-05-25 18:50   ` [PATCH 09/20] ipc/sem: Avoid ipc_rcu_alloc() Manfred Spraul
2017-05-25 18:50   ` [PATCH 10/20] ipc/shm: " Manfred Spraul
2017-05-25 18:50   ` [PATCH 11/20] ipc/msg: " Manfred Spraul
2017-05-25 18:50   ` [PATCH 12/20] ipc/util: Drop ipc_rcu_alloc() Manfred Spraul
2017-05-25 18:51   ` [PATCH 13/20] ipc/sem.c: Avoid ipc_rcu_putref for failed ipc_addid() Manfred Spraul
2017-05-25 18:51   ` [PATCH 14/20] ipc/shm.c: " Manfred Spraul
2017-05-25 18:51   ` [PATCH 15/20] ipc/msg.c: " Manfred Spraul
2017-05-25 18:51   ` [PATCH 16/20] ipc: Move atomic_set() to where it is needed Manfred Spraul
2017-05-25 18:51   ` [PATCH 17/20] ipc/shm: Remove special shm_alloc/free Manfred Spraul
2017-05-25 18:51   ` [PATCH 18/20] ipc/msg: Remove special msg_alloc/free Manfred Spraul
2017-05-25 18:51   ` [PATCH 19/20] ipc/sem: Drop __sem_free() Manfred Spraul
2017-05-25 18:51   ` [PATCH 20/20] ipc/util.h: Update documentation for ipc_getref() and ipc_putref() Manfred Spraul
2017-05-25 19:45   ` [PATCH 0/20 V3] Misc cleanups for ipc Kees Cook
2017-05-26  1:56     ` Manfred Spraul

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=CAGXu5jJt9yB-4tbcBOBwqFB5STP7AVYFt-THpjHsCJNL7jdvKA@mail.gmail.com \
    --to=keescook@chromium.org \
    --cc=akpm@linux-foundation.org \
    --cc=dave@stgolabs.net \
    --cc=fabf@skynet.be \
    --cc=linux-kernel@vger.kernel.org \
    --cc=manfred@colorfullife.com \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.