linux-kernel - Re: [PATCH] ipc: constify ipc

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: <1396275156.2507.4.camel@buesod1.americas.hpqcorp.net>
Date:	Mon, 31 Mar 2014 07:12:36 -0700
From:	Davidlohr Bueso <davidlohr@...com>
To:	Mathias Krause <minipli@...glemail.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	PaX Team <pageexec@...email.hu>,
	Nadia Derbey <Nadia.Derbey@...l.net>,
	Manfred Spraul <manfred@...orfullife.com>
Subject: Re: [PATCH] ipc: constify ipc_ops

On Mon, 2014-03-31 at 10:28 +0200, Mathias Krause wrote:
> On 30 March 2014 23:47, Davidlohr Bueso <davidlohr@...com> wrote:
> > On Sun, 2014-03-30 at 15:35 +0200, Mathias Krause wrote:
> >> There is no need to recreate the very same ipc_ops structure on every
> >> kernel entry for msgget/semget/shmget. Just declare it static and be
> >> done with it.
> >> While at it, constify it as we don't modify the structure at runtime.
> >
> > Seems reasonable.
> >
> >>
> >> Found in the PaX patch, written by the PaX Team.
> >>
> >> Cc: PaX Team <pageexec@...email.hu>
> >> Cc: Nadia Derbey <Nadia.Derbey@...l.net>
> >> Cc: Andrew Morton <akpm@...ux-foundation.org>
> >> Signed-off-by: Mathias Krause <minipli@...glemail.com>
> >
> > In the future, for ipc changes, please CC me and Manfred as well.
> 
> As there is no maintainer entry for ipc/ in the MAINTAINERS file I
> looked at the recent commit history of ipc/ and found that most of the
> patches went in via Andrew and then Linus. But it's true, you and
> Manfred have quite some patches in the recent past. Though, having an
> official maintaner for ipc/ would accelerate the gyrating phase of
> whom to send the patches to in the future. Are you volunteering? :)
> 
> >
> > One comment below, otherwise:
> > Acked-by: Davidlohr Bueso <davidlohr@...com>
> >
> >> ---
> >>  ipc/msg.c  |    9 ++++-----
> >>  ipc/sem.c  |   10 +++++-----
> >>  ipc/shm.c  |   10 +++++-----
> >>  ipc/util.c |    8 ++++----
> >>  ipc/util.h |    2 +-
> >>  5 files changed, 19 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/ipc/msg.c b/ipc/msg.c
> >> index 649853105a..35e4018de5 100644
> >> --- a/ipc/msg.c
> >> +++ b/ipc/msg.c
> >> @@ -306,15 +306,14 @@ static inline int msg_security(struct kern_ipc_perm *ipcp, int msgflg)
> >>  SYSCALL_DEFINE2(msgget, key_t, key, int, msgflg)
> >>  {
> >>       struct ipc_namespace *ns;
> >> -     struct ipc_ops msg_ops;
> >> +     static const struct ipc_ops msg_ops = {
> >> +             .getnew = newque,
> >> +             .associate = msg_security,
> >
> > For completeness, please add .more_checks = NULL as well.
> 
> The C standard already ensures that .more_checks is initialized with
> NULL. So I don't see any benefit from doing it explicitly.
> We're not initializing global/static variables holding NULL pointers
> in the majority of the code base either, so this is just "best
> practice".

Hence the word _completeness_.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/