linux-kernel - Re: [PATCH 1/4] sysvipc: unteach ids->next_id for !CHECKPOINT

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] [day] [month] [year] [list]

Message-Id: <20171003150358.d9b902d2e3e1489f425fd38c@linux-foundation.org>
Date:   Tue, 3 Oct 2017 15:03:58 -0700
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Davidlohr Bueso <dave@...olabs.net>
Cc:     manfred@...orfullife.com, linux-kernel@...r.kernel.org,
        Davidlohr Bueso <dbueso@...e.de>
Subject: Re: [PATCH 1/4] sysvipc: unteach ids->next_id for
 !CHECKPOINT_RESTORE

On Thu, 31 Aug 2017 10:20:46 -0700 Davidlohr Bueso <dave@...olabs.net> wrote:

> The next_id object-allocation functionality was introduced in
> 03f595668017f (ipc: add sysctl to specify desired next object id).
> Given that these new entries are _only_ exported under the
> CONFIG_CHECKPOINT_RESTORE option, there is no point for the
> common case to even know about ->next_id. As such rewrite
> ipc_buildid() such that it can do away with the field as well as
> unnecessary branches when adding a new identifier. The end result
> also better differentiates both cases, so the code ends up being
> cleaner; albeit the small duplications regarding the default case.
> 
> ...
>
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> +/*
> + * Specify desired id for next allocated IPC object.
> + */
> +#define ipc_idr_alloc(ids, new)						\
> +	idr_alloc(&(ids)->ipcs_idr, (new),				\
> +		  (ids)->next_id < 0 ? 0: ipcid_to_idx((ids)->next_id), \
> +		  0, GFP_NOWAIT);
> +
> 
> ...
>
> +#else
> +#define ipc_idr_alloc(ids, new)					\
> +	idr_alloc(&(ids)->ipcs_idr, (new), 0, 0, GFP_NOWAIT);
> +
> ...

Not a fan of checkpatch, I see...

--- a/ipc/util.c~sysvipc-unteach-ids-next_id-for-checkpoint_restore-checkpatch-fixes
+++ a/ipc/util.c
@@ -223,8 +223,8 @@ int ipc_get_maxid(struct ipc_ids *ids)
  */
 #define ipc_idr_alloc(ids, new)						\
 	idr_alloc(&(ids)->ipcs_idr, (new),				\
-		  (ids)->next_id < 0 ? 0: ipcid_to_idx((ids)->next_id), \
-		  0, GFP_NOWAIT);
+		  (ids)->next_id < 0 ? 0 : ipcid_to_idx((ids)->next_id),\
+		  0, GFP_NOWAIT)
 
 static inline int ipc_buildid(int id, struct ipc_ids *ids,
 			      struct kern_ipc_perm *new)
@@ -243,7 +243,7 @@ static inline int ipc_buildid(int id, st
 
 #else
 #define ipc_idr_alloc(ids, new)					\
-	idr_alloc(&(ids)->ipcs_idr, (new), 0, 0, GFP_NOWAIT);
+	idr_alloc(&(ids)->ipcs_idr, (new), 0, 0, GFP_NOWAIT)
 
 static inline int ipc_buildid(int id, struct ipc_ids *ids,
 			      struct kern_ipc_perm *new)

Did these "functions" *have* to be implemented in cpp?  Wouldn't they
be better in C?