* Re: [PATCH v5 bpf-next 06/10] tracepoint: compute num_args at build time
From: Steven Rostedt @ 2018-03-26 15:02 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: davem, daniel, torvalds, peterz, netdev, kernel-team, linux-api,
Mathieu Desnoyers
In-Reply-To: <20180324023038.938665-7-ast@fb.com>
On Fri, 23 Mar 2018 19:30:34 -0700
Alexei Starovoitov <ast@fb.com> wrote:
> From: Alexei Starovoitov <ast@kernel.org>
>
> add fancy macro to compute number of arguments passed into tracepoint
> at compile time and store it as part of 'struct tracepoint'.
> The number is necessary to check safety of bpf program access that
> is coming in subsequent patch.
>
> for_each_tracepoint_range() api has no users inside the kernel.
> Make it more useful with ability to stop for_each() loop depending
> via callback return value.
> In such form it's used in subsequent patch.
I believe this is used by LTTng.
-- Steve
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> include/linux/tracepoint-defs.h | 1 +
> include/linux/tracepoint.h | 28 +++++++++++++++++++---------
> include/trace/define_trace.h | 14 +++++++-------
> kernel/tracepoint.c | 27 ++++++++++++++++-----------
> 4 files changed, 43 insertions(+), 27 deletions(-)
>
> diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
> index 64ed7064f1fa..39a283c61c51 100644
> --- a/include/linux/tracepoint-defs.h
> +++ b/include/linux/tracepoint-defs.h
> @@ -33,6 +33,7 @@ struct tracepoint {
> int (*regfunc)(void);
> void (*unregfunc)(void);
> struct tracepoint_func __rcu *funcs;
> + u32 num_args;
> };
>
> #endif
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index c94f466d57ef..2194e7c31484 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -40,9 +40,19 @@ tracepoint_probe_register_prio(struct tracepoint *tp, void *probe, void *data,
> int prio);
> extern int
> tracepoint_probe_unregister(struct tracepoint *tp, void *probe, void *data);
> -extern void
> -for_each_kernel_tracepoint(void (*fct)(struct tracepoint *tp, void *priv),
> - void *priv);
> +
> +#ifdef CONFIG_TRACEPOINTS
> +void *
> +for_each_kernel_tracepoint(void *(*fct)(struct tracepoint *tp, void *priv),
> + void *priv);
> +#else
> +static inline void *
> +for_each_kernel_tracepoint(void *(*fct)(struct tracepoint *tp, void *priv),
> + void *priv)
> +{
> + return NULL;
> +}
> +#endif
>
> #ifdef CONFIG_MODULES
> struct tp_module {
> @@ -230,18 +240,18 @@ extern void syscall_unregfunc(void);
> * structures, so we create an array of pointers that will be used for iteration
> * on the tracepoints.
> */
> -#define DEFINE_TRACE_FN(name, reg, unreg) \
> +#define DEFINE_TRACE_FN(name, reg, unreg, num_args) \
> static const char __tpstrtab_##name[] \
> __attribute__((section("__tracepoints_strings"))) = #name; \
> struct tracepoint __tracepoint_##name \
> __attribute__((section("__tracepoints"))) = \
> - { __tpstrtab_##name, STATIC_KEY_INIT_FALSE, reg, unreg, NULL };\
> + { __tpstrtab_##name, STATIC_KEY_INIT_FALSE, reg, unreg, NULL, num_args };\
> static struct tracepoint * const __tracepoint_ptr_##name __used \
> __attribute__((section("__tracepoints_ptrs"))) = \
> &__tracepoint_##name;
>
> -#define DEFINE_TRACE(name) \
> - DEFINE_TRACE_FN(name, NULL, NULL);
> +#define DEFINE_TRACE(name, num_args) \
> + DEFINE_TRACE_FN(name, NULL, NULL, num_args);
>
> #define EXPORT_TRACEPOINT_SYMBOL_GPL(name) \
> EXPORT_SYMBOL_GPL(__tracepoint_##name)
> @@ -275,8 +285,8 @@ extern void syscall_unregfunc(void);
> return false; \
> }
>
> -#define DEFINE_TRACE_FN(name, reg, unreg)
> -#define DEFINE_TRACE(name)
> +#define DEFINE_TRACE_FN(name, reg, unreg, num_args)
> +#define DEFINE_TRACE(name, num_args)
> #define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
> #define EXPORT_TRACEPOINT_SYMBOL(name)
>
> diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
> index d9e3d4aa3f6e..96b22ace9ae7 100644
> --- a/include/trace/define_trace.h
> +++ b/include/trace/define_trace.h
> @@ -25,7 +25,7 @@
>
> #undef TRACE_EVENT
> #define TRACE_EVENT(name, proto, args, tstruct, assign, print) \
> - DEFINE_TRACE(name)
> + DEFINE_TRACE(name, COUNT_ARGS(args))
>
> #undef TRACE_EVENT_CONDITION
> #define TRACE_EVENT_CONDITION(name, proto, args, cond, tstruct, assign, print) \
> @@ -39,24 +39,24 @@
> #undef TRACE_EVENT_FN
> #define TRACE_EVENT_FN(name, proto, args, tstruct, \
> assign, print, reg, unreg) \
> - DEFINE_TRACE_FN(name, reg, unreg)
> + DEFINE_TRACE_FN(name, reg, unreg, COUNT_ARGS(args))
>
> #undef TRACE_EVENT_FN_COND
> #define TRACE_EVENT_FN_COND(name, proto, args, cond, tstruct, \
> assign, print, reg, unreg) \
> - DEFINE_TRACE_FN(name, reg, unreg)
> + DEFINE_TRACE_FN(name, reg, unreg, COUNT_ARGS(args))
>
> #undef DEFINE_EVENT
> #define DEFINE_EVENT(template, name, proto, args) \
> - DEFINE_TRACE(name)
> + DEFINE_TRACE(name, COUNT_ARGS(args))
>
> #undef DEFINE_EVENT_FN
> #define DEFINE_EVENT_FN(template, name, proto, args, reg, unreg) \
> - DEFINE_TRACE_FN(name, reg, unreg)
> + DEFINE_TRACE_FN(name, reg, unreg, COUNT_ARGS(args))
>
> #undef DEFINE_EVENT_PRINT
> #define DEFINE_EVENT_PRINT(template, name, proto, args, print) \
> - DEFINE_TRACE(name)
> + DEFINE_TRACE(name, COUNT_ARGS(args))
>
> #undef DEFINE_EVENT_CONDITION
> #define DEFINE_EVENT_CONDITION(template, name, proto, args, cond) \
> @@ -64,7 +64,7 @@
>
> #undef DECLARE_TRACE
> #define DECLARE_TRACE(name, proto, args) \
> - DEFINE_TRACE(name)
> + DEFINE_TRACE(name, COUNT_ARGS(args))
>
> #undef TRACE_INCLUDE
> #undef __TRACE_INCLUDE
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index 671b13457387..3f2dc5738c2b 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -502,17 +502,22 @@ static __init int init_tracepoints(void)
> __initcall(init_tracepoints);
> #endif /* CONFIG_MODULES */
>
> -static void for_each_tracepoint_range(struct tracepoint * const *begin,
> - struct tracepoint * const *end,
> - void (*fct)(struct tracepoint *tp, void *priv),
> - void *priv)
> +static void *for_each_tracepoint_range(struct tracepoint * const *begin,
> + struct tracepoint * const *end,
> + void *(*fct)(struct tracepoint *tp, void *priv),
> + void *priv)
> {
> struct tracepoint * const *iter;
> + void *ret;
>
> if (!begin)
> - return;
> - for (iter = begin; iter < end; iter++)
> - fct(*iter, priv);
> + return NULL;
> + for (iter = begin; iter < end; iter++) {
> + ret = fct(*iter, priv);
> + if (ret)
> + return ret;
> + }
> + return NULL;
> }
>
> /**
> @@ -520,11 +525,11 @@ static void for_each_tracepoint_range(struct tracepoint * const *begin,
> * @fct: callback
> * @priv: private data
> */
> -void for_each_kernel_tracepoint(void (*fct)(struct tracepoint *tp, void *priv),
> - void *priv)
> +void *for_each_kernel_tracepoint(void *(*fct)(struct tracepoint *tp, void *priv),
> + void *priv)
> {
> - for_each_tracepoint_range(__start___tracepoints_ptrs,
> - __stop___tracepoints_ptrs, fct, priv);
> + return for_each_tracepoint_range(__start___tracepoints_ptrs,
> + __stop___tracepoints_ptrs, fct, priv);
> }
> EXPORT_SYMBOL_GPL(for_each_kernel_tracepoint);
>
^ permalink raw reply
* Re: [PATCH v5 bpf-next 00/10] bpf, tracing: introduce bpf raw tracepoints
From: Daniel Borkmann @ 2018-03-26 8:28 UTC (permalink / raw)
To: Alexei Starovoitov, davem
Cc: torvalds, peterz, rostedt, netdev, kernel-team, linux-api
In-Reply-To: <20180324023038.938665-1-ast@fb.com>
On 03/24/2018 03:30 AM, Alexei Starovoitov wrote:
> From: Alexei Starovoitov <ast@kernel.org>
>
> v4->v5:
> - adopted Daniel's fancy REPEAT macro in bpf_trace.c in patch 7
>
> v3->v4:
> - adopted Linus's CAST_TO_U64 macro to cast any integer, pointer, or small
> struct to u64. That nicely reduced the size of patch 1
>
> v2->v3:
> - with Linus's suggestion introduced generic COUNT_ARGS and CONCATENATE macros
> (or rather moved them from apparmor)
> that cleaned up patches 6 and 7
> - added patch 4 to refactor trace_iwlwifi_dev_ucode_error() from 17 args to 4
> Now any tracepoint with >12 args will have build error
>
> v1->v2:
> - simplified api by combing bpf_raw_tp_open(name) + bpf_attach(prog_fd) into
> bpf_raw_tp_open(name, prog_fd) as suggested by Daniel.
> That simplifies bpf_detach as well which is now simple close() of fd.
> - fixed memory leak in error path which was spotted by Daniel.
> - fixed bpf_get_stackid(), bpf_perf_event_output() called from raw tracepoints
> - added more tests
> - fixed allyesconfig build caught by buildbot
>
> v1:
> This patch set is a different way to address the pressing need to access
> task_struct pointers in sched tracepoints from bpf programs.
>
> The first approach simply added these pointers to sched tracepoints:
> https://lkml.org/lkml/2017/12/14/753
> which Peter nacked.
> Few options were discussed and eventually the discussion converged on
> doing bpf specific tracepoint_probe_register() probe functions.
> Details here:
> https://lkml.org/lkml/2017/12/20/929
>
> Patch 1 is kernel wide cleanup of pass-struct-by-value into
> pass-struct-by-reference into tracepoints.
>
> Patches 2 and 3 are minor cleanups to address allyesconfig build
>
> Patch 4 refactor trace_iwlwifi_dev_ucode_error from 17 to 4 args
>
> Patch 5 introduces COUNT_ARGS macro
>
> Patch 6 minor prep work to expose number of arguments passed
> into tracepoints.
>
> Patch 7 introduces BPF_RAW_TRACEPOINT api.
> the auto-cleanup and multiple concurrent users are must have
> features of tracing api. For bpf raw tracepoints it looks like:
> // load bpf prog with BPF_PROG_TYPE_RAW_TRACEPOINT type
> prog_fd = bpf_prog_load(...);
>
> // receive anon_inode fd for given bpf_raw_tracepoint
> // and attach bpf program to it
> raw_tp_fd = bpf_raw_tracepoint_open("xdp_exception", prog_fd);
>
> Ctrl-C of tracing daemon or cmdline tool will automatically
> detach bpf program, unload it and unregister tracepoint probe.
> More details in patch 7.
>
> Patch 8 - trivial support in libbpf
> Patches 9, 10 - user space tests
>
> samples/bpf/test_overhead performance on 1 cpu:
>
> tracepoint base kprobe+bpf tracepoint+bpf raw_tracepoint+bpf
> task_rename 1.1M 769K 947K 1.0M
> urandom_read 789K 697K 750K 755K
Applied to bpf-next, thanks Alexei!
^ permalink raw reply
* Re: [PATCH v2 bpf-next 5/8] bpf: introduce BPF_RAW_TRACEPOINT
From: Daniel Borkmann @ 2018-03-26 7:53 UTC (permalink / raw)
To: Alexei Starovoitov, davem
Cc: torvalds, peterz, rostedt, netdev, kernel-team, linux-api
In-Reply-To: <cea19b0f-0420-0620-2240-954a81e22659@fb.com>
On 03/24/2018 02:43 AM, Alexei Starovoitov wrote:
> On 3/23/18 5:58 PM, Alexei Starovoitov wrote:
>> On 3/23/18 4:13 PM, Daniel Borkmann wrote:
>>> On 03/22/2018 04:41 PM, Alexei Starovoitov wrote:
>>>> On 3/22/18 2:43 AM, Daniel Borkmann wrote:
>>>>> On 03/21/2018 07:54 PM, Alexei Starovoitov wrote:
[...]
>>>> I picked 6 as a good compromise and used it twice in bpf_trace_run1x()
>>>> Similar thing possible for u64 arg1, u64 arg2, ...
>>>> but it will be harder to read.
>>>> Looking forward what you can come up with.
>>>
>>> Just took a quick look, so the below one would work for generating the
>>> signature and function. I did till 9 here:
>>>
>>> #define UNPACK(...) __VA_ARGS__
>>> #define REPEAT_1(FN, DL, X, ...) FN(X)
>>> #define REPEAT_2(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_1(FN, DL,
>>> __VA_ARGS__)
>>> #define REPEAT_3(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_2(FN, DL,
>>> __VA_ARGS__)
>>> #define REPEAT_4(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_3(FN, DL,
>>> __VA_ARGS__)
>>> #define REPEAT_5(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_4(FN, DL,
>>> __VA_ARGS__)
>>> #define REPEAT_6(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_5(FN, DL,
>>> __VA_ARGS__)
>>> #define REPEAT_7(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_6(FN, DL,
>>> __VA_ARGS__)
>>> #define REPEAT_8(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_7(FN, DL,
>>> __VA_ARGS__)
>>> #define REPEAT_9(FN, DL, X, ...) FN(X) UNPACK DL REPEAT_8(FN, DL,
>>> __VA_ARGS__)
>>> #define REPEAT(X, FN, DL, ...) REPEAT_##X(FN, DL, __VA_ARGS__)
>>>
>>> #define SARG(X) u64 arg##X
>>> #define COPY(X) args[X] = arg##X
>>>
>>> #define __DL_COM (,)
>>> #define __DL_SEM (;)
>>>
>>> #define __SEQ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
>>>
>>> #define BPF_TRACE_DECL_x(x) \
>>> void bpf_trace_run##x(struct bpf_prog *prog, \
>>> REPEAT(x, SARG, __DL_COM, __SEQ))
>>> #define BPF_TRACE_DEFN_x(x) \
>>> void bpf_trace_run##x(struct bpf_prog *prog, \
>>> REPEAT(x, SARG, __DL_COM, __SEQ)) \
>>> { \
>>> u64 args[x]; \
>>> REPEAT(x, COPY, __DL_SEM, __SEQ); \
>>> __bpf_trace_run(prog, args); \
>>> } \
>>> EXPORT_SYMBOL_GPL(bpf_trace_run##x)
>>>
>>> So doing a ...
>>>
>>> BPF_TRACE_DECL_x(5);
>>> BPF_TRACE_DEFN_x(5);
>>
>> interestingly that in addition to above defining
>> #define __REPEAT(X, FN, DL, ...) REPEAT_##X(FN, DL, __VA_ARGS__)
>> to allow recursive expansion and doing
>> __REPEAT(12, BPF_TRACE_DECL_x, __DL_SEM, __SEQ_1_12);
>> almost works...
>> I'm guessing it's hitting preprocessor internal limit on
>> number of expressions to expand.
>> It expands 1-6 nicely and 7-12 are partially expanded :)
>
> it's not the limit I'm hitting, but self referential issue.
> Exactly half gets expanded.
> I don't think there is an easy workaround other
> than duplicating the whole chain of REPEAT macro twice
> with slightly different name.
Hmm, that is kind of annoying, probably worth filing a bug on gcc, we still
won't be able to use it near term though.
Given it expands just fine from 1-6 arguments, I think Steven had a good
choice on upper limit of 6 args then (including build error with above). ;-)
Thanks,
Daniel
^ permalink raw reply
* Re: [REVIEW][PATCH 13/11] ipc/smack: Tidy up from the change in type of the ipc security hooks
From: Casey Schaufler @ 2018-03-25 0:05 UTC (permalink / raw)
To: Eric W. Biederman, Linux Containers
Cc: Paul Moore, jannh-hpIqsD4AKlfQT0dZR+AlfA,
khlebnikov-XoJtRXgx1JseBXzfvpsJ4g,
linux-api-u79uwXL29TY76Z2rM5mHXA,
serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA, oleg-H+wXaHxf7aLQT0dZR+AlfA,
esyr-H+wXaHxf7aLQT0dZR+AlfA, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
luto-DgEjT+Ai2ygdnm+yROfE0A,
prakash.sangappa-QHcLZuEGTsvQT0dZR+AlfA, Casey Schaufler,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Nagarathnam Muthusamy,
Pavel Emelyanov
In-Reply-To: <87y3iikp1y.fsf_-_-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
On 3/23/2018 10:42 PM, Eric W. Biederman wrote:
> Rename the variables shp, sma, msq to isp. As that is how the code already
> refers to those variables.
Thanks. It's important to keep the code readable.
> Collapse smack_of_shm, smack_of_sem, and smack_of_msq into smack_of_ipc,
> as the three functions had become completely identical.
Thanks. Completely reasonable and correct.
> Collapse smack_shm_alloc_security, smack_sem_alloc_security and
> smack_msg_queue_alloc_security into smack_ipc_alloc_security as the
> three functions had become identical.
>
> Collapse smack_shm_free_security, smack_sem_free_security and
> smack_msg_queue_free_security into smack_ipc_free_security as the three
> functions had become identical.
This is reasonable but unprecedented. Nowhere else is the
same function used to supply multiple LSM hooks. Does anyone
out there see a reason not to do this?
> Requested-by: Casey Schaufler <casey-iSGtlc1asvQWG2LlvL+J4A@public.gmane.org>
> Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
> ---
> security/smack/smack_lsm.c | 197 +++++++++++++--------------------------------
> 1 file changed, 58 insertions(+), 139 deletions(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index d960c2ea8d79..0735b8db158b 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -2945,25 +2945,24 @@ static void smack_msg_msg_free_security(struct msg_msg *msg)
> }
>
> /**
> - * smack_of_shm - the smack pointer for the shm
> - * @shp: the object
> + * smack_of_ipc - the smack pointer for the ipc
> + * @isp: the object
> *
> * Returns a pointer to the smack value
> */
> -static struct smack_known *smack_of_shm(struct kern_ipc_perm *shp)
> +static struct smack_known *smack_of_ipc(struct kern_ipc_perm *isp)
> {
> - return (struct smack_known *)shp->security;
> + return (struct smack_known *)isp->security;
> }
>
> /**
> - * smack_shm_alloc_security - Set the security blob for shm
> - * @shp: the object
> + * smack_ipc_alloc_security - Set the security blob for ipc
> + * @isp: the object
> *
> * Returns 0
> */
> -static int smack_shm_alloc_security(struct kern_ipc_perm *shp)
> +static int smack_ipc_alloc_security(struct kern_ipc_perm *isp)
> {
> - struct kern_ipc_perm *isp = shp;
> struct smack_known *skp = smk_of_current();
>
> isp->security = skp;
> @@ -2971,34 +2970,32 @@ static int smack_shm_alloc_security(struct kern_ipc_perm *shp)
> }
>
> /**
> - * smack_shm_free_security - Clear the security blob for shm
> - * @shp: the object
> + * smack_ipc_free_security - Clear the security blob for ipc
> + * @isp: the object
> *
> * Clears the blob pointer
> */
> -static void smack_shm_free_security(struct kern_ipc_perm *shp)
> +static void smack_ipc_free_security(struct kern_ipc_perm *isp)
> {
> - struct kern_ipc_perm *isp = shp;
> -
> isp->security = NULL;
> }
>
> /**
> * smk_curacc_shm : check if current has access on shm
> - * @shp : the object
> + * @isp : the object
> * @access : access requested
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smk_curacc_shm(struct kern_ipc_perm *shp, int access)
> +static int smk_curacc_shm(struct kern_ipc_perm *isp, int access)
> {
> - struct smack_known *ssp = smack_of_shm(shp);
> + struct smack_known *ssp = smack_of_ipc(isp);
> struct smk_audit_info ad;
> int rc;
>
> #ifdef CONFIG_AUDIT
> smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_IPC);
> - ad.a.u.ipc_id = shp->id;
> + ad.a.u.ipc_id = isp->id;
> #endif
> rc = smk_curacc(ssp, access, &ad);
> rc = smk_bu_current("shm", ssp, access, rc);
> @@ -3007,27 +3004,27 @@ static int smk_curacc_shm(struct kern_ipc_perm *shp, int access)
>
> /**
> * smack_shm_associate - Smack access check for shm
> - * @shp: the object
> + * @isp: the object
> * @shmflg: access requested
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smack_shm_associate(struct kern_ipc_perm *shp, int shmflg)
> +static int smack_shm_associate(struct kern_ipc_perm *isp, int shmflg)
> {
> int may;
>
> may = smack_flags_to_may(shmflg);
> - return smk_curacc_shm(shp, may);
> + return smk_curacc_shm(isp, may);
> }
>
> /**
> * smack_shm_shmctl - Smack access check for shm
> - * @shp: the object
> + * @isp: the object
> * @cmd: what it wants to do
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smack_shm_shmctl(struct kern_ipc_perm *shp, int cmd)
> +static int smack_shm_shmctl(struct kern_ipc_perm *isp, int cmd)
> {
> int may;
>
> @@ -3051,81 +3048,42 @@ static int smack_shm_shmctl(struct kern_ipc_perm *shp, int cmd)
> default:
> return -EINVAL;
> }
> - return smk_curacc_shm(shp, may);
> + return smk_curacc_shm(isp, may);
> }
>
> /**
> * smack_shm_shmat - Smack access for shmat
> - * @shp: the object
> + * @isp: the object
> * @shmaddr: unused
> * @shmflg: access requested
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smack_shm_shmat(struct kern_ipc_perm *shp, char __user *shmaddr,
> +static int smack_shm_shmat(struct kern_ipc_perm *ipc, char __user *shmaddr,
> int shmflg)
> {
> int may;
>
> may = smack_flags_to_may(shmflg);
> - return smk_curacc_shm(shp, may);
> -}
> -
> -/**
> - * smack_of_sem - the smack pointer for the sem
> - * @sma: the object
> - *
> - * Returns a pointer to the smack value
> - */
> -static struct smack_known *smack_of_sem(struct kern_ipc_perm *sma)
> -{
> - return (struct smack_known *)sma->security;
> -}
> -
> -/**
> - * smack_sem_alloc_security - Set the security blob for sem
> - * @sma: the object
> - *
> - * Returns 0
> - */
> -static int smack_sem_alloc_security(struct kern_ipc_perm *sma)
> -{
> - struct kern_ipc_perm *isp = sma;
> - struct smack_known *skp = smk_of_current();
> -
> - isp->security = skp;
> - return 0;
> -}
> -
> -/**
> - * smack_sem_free_security - Clear the security blob for sem
> - * @sma: the object
> - *
> - * Clears the blob pointer
> - */
> -static void smack_sem_free_security(struct kern_ipc_perm *sma)
> -{
> - struct kern_ipc_perm *isp = sma;
> -
> - isp->security = NULL;
> + return smk_curacc_shm(ipc, may);
> }
>
> /**
> * smk_curacc_sem : check if current has access on sem
> - * @sma : the object
> + * @isp : the object
> * @access : access requested
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smk_curacc_sem(struct kern_ipc_perm *sma, int access)
> +static int smk_curacc_sem(struct kern_ipc_perm *isp, int access)
> {
> - struct smack_known *ssp = smack_of_sem(sma);
> + struct smack_known *ssp = smack_of_ipc(isp);
> struct smk_audit_info ad;
> int rc;
>
> #ifdef CONFIG_AUDIT
> smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_IPC);
> - ad.a.u.ipc_id = sma->id;
> + ad.a.u.ipc_id = isp->id;
> #endif
> rc = smk_curacc(ssp, access, &ad);
> rc = smk_bu_current("sem", ssp, access, rc);
> @@ -3134,27 +3092,27 @@ static int smk_curacc_sem(struct kern_ipc_perm *sma, int access)
>
> /**
> * smack_sem_associate - Smack access check for sem
> - * @sma: the object
> + * @isp: the object
> * @semflg: access requested
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smack_sem_associate(struct kern_ipc_perm *sma, int semflg)
> +static int smack_sem_associate(struct kern_ipc_perm *isp, int semflg)
> {
> int may;
>
> may = smack_flags_to_may(semflg);
> - return smk_curacc_sem(sma, may);
> + return smk_curacc_sem(isp, may);
> }
>
> /**
> * smack_sem_shmctl - Smack access check for sem
> - * @sma: the object
> + * @isp: the object
> * @cmd: what it wants to do
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smack_sem_semctl(struct kern_ipc_perm *sma, int cmd)
> +static int smack_sem_semctl(struct kern_ipc_perm *isp, int cmd)
> {
> int may;
>
> @@ -3184,12 +3142,12 @@ static int smack_sem_semctl(struct kern_ipc_perm *sma, int cmd)
> return -EINVAL;
> }
>
> - return smk_curacc_sem(sma, may);
> + return smk_curacc_sem(isp, may);
> }
>
> /**
> * smack_sem_semop - Smack checks of semaphore operations
> - * @sma: the object
> + * @isp: the object
> * @sops: unused
> * @nsops: unused
> * @alter: unused
> @@ -3198,67 +3156,28 @@ static int smack_sem_semctl(struct kern_ipc_perm *sma, int cmd)
> *
> * Returns 0 if access is allowed, error code otherwise
> */
> -static int smack_sem_semop(struct kern_ipc_perm *sma, struct sembuf *sops,
> +static int smack_sem_semop(struct kern_ipc_perm *isp, struct sembuf *sops,
> unsigned nsops, int alter)
> {
> - return smk_curacc_sem(sma, MAY_READWRITE);
> -}
> -
> -/**
> - * smack_msg_alloc_security - Set the security blob for msg
> - * @msq: the object
> - *
> - * Returns 0
> - */
> -static int smack_msg_queue_alloc_security(struct kern_ipc_perm *msq)
> -{
> - struct kern_ipc_perm *kisp = msq;
> - struct smack_known *skp = smk_of_current();
> -
> - kisp->security = skp;
> - return 0;
> -}
> -
> -/**
> - * smack_msg_free_security - Clear the security blob for msg
> - * @msq: the object
> - *
> - * Clears the blob pointer
> - */
> -static void smack_msg_queue_free_security(struct kern_ipc_perm *msq)
> -{
> - struct kern_ipc_perm *kisp = msq;
> -
> - kisp->security = NULL;
> -}
> -
> -/**
> - * smack_of_msq - the smack pointer for the msq
> - * @msq: the object
> - *
> - * Returns a pointer to the smack label entry
> - */
> -static struct smack_known *smack_of_msq(struct kern_ipc_perm *msq)
> -{
> - return (struct smack_known *)msq->security;
> + return smk_curacc_sem(isp, MAY_READWRITE);
> }
>
> /**
> * smk_curacc_msq : helper to check if current has access on msq
> - * @msq : the msq
> + * @isp : the msq
> * @access : access requested
> *
> * return 0 if current has access, error otherwise
> */
> -static int smk_curacc_msq(struct kern_ipc_perm *msq, int access)
> +static int smk_curacc_msq(struct kern_ipc_perm *isp, int access)
> {
> - struct smack_known *msp = smack_of_msq(msq);
> + struct smack_known *msp = smack_of_ipc(isp);
> struct smk_audit_info ad;
> int rc;
>
> #ifdef CONFIG_AUDIT
> smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_IPC);
> - ad.a.u.ipc_id = msq->id;
> + ad.a.u.ipc_id = isp->id;
> #endif
> rc = smk_curacc(msp, access, &ad);
> rc = smk_bu_current("msq", msp, access, rc);
> @@ -3267,27 +3186,27 @@ static int smk_curacc_msq(struct kern_ipc_perm *msq, int access)
>
> /**
> * smack_msg_queue_associate - Smack access check for msg_queue
> - * @msq: the object
> + * @isp: the object
> * @msqflg: access requested
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smack_msg_queue_associate(struct kern_ipc_perm *msq, int msqflg)
> +static int smack_msg_queue_associate(struct kern_ipc_perm *isp, int msqflg)
> {
> int may;
>
> may = smack_flags_to_may(msqflg);
> - return smk_curacc_msq(msq, may);
> + return smk_curacc_msq(isp, may);
> }
>
> /**
> * smack_msg_queue_msgctl - Smack access check for msg_queue
> - * @msq: the object
> + * @isp: the object
> * @cmd: what it wants to do
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smack_msg_queue_msgctl(struct kern_ipc_perm *msq, int cmd)
> +static int smack_msg_queue_msgctl(struct kern_ipc_perm *isp, int cmd)
> {
> int may;
>
> @@ -3310,29 +3229,29 @@ static int smack_msg_queue_msgctl(struct kern_ipc_perm *msq, int cmd)
> return -EINVAL;
> }
>
> - return smk_curacc_msq(msq, may);
> + return smk_curacc_msq(isp, may);
> }
>
> /**
> * smack_msg_queue_msgsnd - Smack access check for msg_queue
> - * @msq: the object
> + * @isp: the object
> * @msg: unused
> * @msqflg: access requested
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smack_msg_queue_msgsnd(struct kern_ipc_perm *msq, struct msg_msg *msg,
> +static int smack_msg_queue_msgsnd(struct kern_ipc_perm *isp, struct msg_msg *msg,
> int msqflg)
> {
> int may;
>
> may = smack_flags_to_may(msqflg);
> - return smk_curacc_msq(msq, may);
> + return smk_curacc_msq(isp, may);
> }
>
> /**
> * smack_msg_queue_msgsnd - Smack access check for msg_queue
> - * @msq: the object
> + * @isp: the object
> * @msg: unused
> * @target: unused
> * @type: unused
> @@ -3340,10 +3259,10 @@ static int smack_msg_queue_msgsnd(struct kern_ipc_perm *msq, struct msg_msg *msg
> *
> * Returns 0 if current has read and write access, error code otherwise
> */
> -static int smack_msg_queue_msgrcv(struct kern_ipc_perm *msq, struct msg_msg *msg,
> +static int smack_msg_queue_msgrcv(struct kern_ipc_perm *isp, struct msg_msg *msg,
> struct task_struct *target, long type, int mode)
> {
> - return smk_curacc_msq(msq, MAY_READWRITE);
> + return smk_curacc_msq(isp, MAY_READWRITE);
> }
>
> /**
> @@ -4756,21 +4675,21 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(msg_msg_alloc_security, smack_msg_msg_alloc_security),
> LSM_HOOK_INIT(msg_msg_free_security, smack_msg_msg_free_security),
>
> - LSM_HOOK_INIT(msg_queue_alloc_security, smack_msg_queue_alloc_security),
> - LSM_HOOK_INIT(msg_queue_free_security, smack_msg_queue_free_security),
> + LSM_HOOK_INIT(msg_queue_alloc_security, smack_ipc_alloc_security),
> + LSM_HOOK_INIT(msg_queue_free_security, smack_ipc_free_security),
> LSM_HOOK_INIT(msg_queue_associate, smack_msg_queue_associate),
> LSM_HOOK_INIT(msg_queue_msgctl, smack_msg_queue_msgctl),
> LSM_HOOK_INIT(msg_queue_msgsnd, smack_msg_queue_msgsnd),
> LSM_HOOK_INIT(msg_queue_msgrcv, smack_msg_queue_msgrcv),
>
> - LSM_HOOK_INIT(shm_alloc_security, smack_shm_alloc_security),
> - LSM_HOOK_INIT(shm_free_security, smack_shm_free_security),
> + LSM_HOOK_INIT(shm_alloc_security, smack_ipc_alloc_security),
> + LSM_HOOK_INIT(shm_free_security, smack_ipc_free_security),
> LSM_HOOK_INIT(shm_associate, smack_shm_associate),
> LSM_HOOK_INIT(shm_shmctl, smack_shm_shmctl),
> LSM_HOOK_INIT(shm_shmat, smack_shm_shmat),
>
> - LSM_HOOK_INIT(sem_alloc_security, smack_sem_alloc_security),
> - LSM_HOOK_INIT(sem_free_security, smack_sem_free_security),
> + LSM_HOOK_INIT(sem_alloc_security, smack_ipc_alloc_security),
> + LSM_HOOK_INIT(sem_free_security, smack_ipc_free_security),
> LSM_HOOK_INIT(sem_associate, smack_sem_associate),
> LSM_HOOK_INIT(sem_semctl, smack_sem_semctl),
> LSM_HOOK_INIT(sem_semop, smack_sem_semop),
^ permalink raw reply
* [PATCH 2/2] mm, memcontrol: Implement memory.swap.events
From: Tejun Heo @ 2018-03-24 16:51 UTC (permalink / raw)
To: hannes, mhocko, vdavydov.dev
Cc: guro, riel, akpm, linux-kernel, kernel-team, cgroups, linux-mm,
Tejun Heo, linux-api
In-Reply-To: <20180324165127.701194-1-tj@kernel.org>
Add swap max and fail events so that userland can monitor and respond
to running out of swap.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: Roman Gushchin <guro@fb.com>
Cc: Rik van Riel <riel@surriel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-api@vger.kernel.org
---
Documentation/cgroup-v2.txt | 16 ++++++++++++++++
include/linux/memcontrol.h | 5 +++++
mm/memcontrol.c | 24 +++++++++++++++++++++++-
3 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/Documentation/cgroup-v2.txt b/Documentation/cgroup-v2.txt
index 74cdeae..b0dda10 100644
--- a/Documentation/cgroup-v2.txt
+++ b/Documentation/cgroup-v2.txt
@@ -1199,6 +1199,22 @@ PAGE_SIZE multiple when read back.
Swap usage hard limit. If a cgroup's swap usage reaches this
limit, anonymous memory of the cgroup will not be swapped out.
+ memory.swap.events
+ A read-only flat-keyed file which exists on non-root cgroups.
+ The following entries are defined. Unless specified
+ otherwise, a value change in this file generates a file
+ modified event.
+
+ max
+ The number of times the cgroup's swap usage was about
+ to go over the max boundary and swap allocation
+ failed.
+
+ fail
+ The number of times swap allocation failed either
+ because of running out of swap system-wide or max
+ limit.
+
Usage Guidelines
~~~~~~~~~~~~~~~~
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 85a8f00..f198339 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -54,6 +54,8 @@ enum memcg_event_item {
MEMCG_HIGH,
MEMCG_MAX,
MEMCG_OOM,
+ MEMCG_SWAP_MAX,
+ MEMCG_SWAP_FAIL,
MEMCG_NR_EVENTS,
};
@@ -202,6 +204,9 @@ struct mem_cgroup {
/* handle for "memory.events" */
struct cgroup_file events_file;
+ /* handle for "memory.swap.events" */
+ struct cgroup_file swap_events_file;
+
/* protect arrays of thresholds */
struct mutex thresholds_lock;
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 9f9c8a7..1a14d4a4 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5987,13 +5987,17 @@ int mem_cgroup_try_charge_swap(struct page *page, swp_entry_t entry)
if (!memcg)
return 0;
- if (!entry.val)
+ if (!entry.val) {
+ mem_cgroup_event(memcg, MEMCG_SWAP_FAIL);
return 0;
+ }
memcg = mem_cgroup_id_get_online(memcg);
if (!mem_cgroup_is_root(memcg) &&
!page_counter_try_charge(&memcg->swap, nr_pages, &counter)) {
+ mem_cgroup_event(memcg, MEMCG_SWAP_MAX);
+ mem_cgroup_event(memcg, MEMCG_SWAP_FAIL);
mem_cgroup_id_put(memcg);
return -ENOMEM;
}
@@ -6131,6 +6135,18 @@ static ssize_t swap_max_write(struct kernfs_open_file *of,
return nbytes;
}
+static int swap_events_show(struct seq_file *m, void *v)
+{
+ struct mem_cgroup *memcg = mem_cgroup_from_css(seq_css(m));
+
+ memcg_stat_flush(memcg);
+
+ seq_printf(m, "max %llu\n", memcg->events[MEMCG_SWAP_MAX]);
+ seq_printf(m, "fail %llu\n", memcg->events[MEMCG_SWAP_FAIL]);
+
+ return 0;
+}
+
static struct cftype swap_files[] = {
{
.name = "swap.current",
@@ -6143,6 +6159,12 @@ static struct cftype swap_files[] = {
.seq_show = swap_max_show,
.write = swap_max_write,
},
+ {
+ .name = "swap.events",
+ .flags = CFTYPE_NOT_ON_ROOT,
+ .file_offset = offsetof(struct mem_cgroup, swap_events_file),
+ .seq_show = swap_events_show,
+ },
{ } /* terminate */
};
--
2.9.5
^ permalink raw reply related
* [REVIEW][PATCH 13/11] ipc/smack: Tidy up from the change in type of the ipc security hooks
From: Eric W. Biederman @ 2018-03-24 5:42 UTC (permalink / raw)
To: Linux Containers
Cc: esyr, jannh, khlebnikov, linux-api, serge.hallyn, linux-kernel,
prakash.sangappa, linux-security-module, luto, oleg, akpm,
Nagarathnam Muthusamy, Pavel Emelyanov
In-Reply-To: <87vadmobdw.fsf_-_@xmission.com>
Rename the variables shp, sma, msq to isp. As that is how the code already
refers to those variables.
Collapse smack_of_shm, smack_of_sem, and smack_of_msq into smack_of_ipc,
as the three functions had become completely identical.
Collapse smack_shm_alloc_security, smack_sem_alloc_security and
smack_msg_queue_alloc_security into smack_ipc_alloc_security as the
three functions had become identical.
Collapse smack_shm_free_security, smack_sem_free_security and
smack_msg_queue_free_security into smack_ipc_free_security as the three
functions had become identical.
Requested-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
security/smack/smack_lsm.c | 197 +++++++++++++--------------------------------
1 file changed, 58 insertions(+), 139 deletions(-)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index d960c2ea8d79..0735b8db158b 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2945,25 +2945,24 @@ static void smack_msg_msg_free_security(struct msg_msg *msg)
}
/**
- * smack_of_shm - the smack pointer for the shm
- * @shp: the object
+ * smack_of_ipc - the smack pointer for the ipc
+ * @isp: the object
*
* Returns a pointer to the smack value
*/
-static struct smack_known *smack_of_shm(struct kern_ipc_perm *shp)
+static struct smack_known *smack_of_ipc(struct kern_ipc_perm *isp)
{
- return (struct smack_known *)shp->security;
+ return (struct smack_known *)isp->security;
}
/**
- * smack_shm_alloc_security - Set the security blob for shm
- * @shp: the object
+ * smack_ipc_alloc_security - Set the security blob for ipc
+ * @isp: the object
*
* Returns 0
*/
-static int smack_shm_alloc_security(struct kern_ipc_perm *shp)
+static int smack_ipc_alloc_security(struct kern_ipc_perm *isp)
{
- struct kern_ipc_perm *isp = shp;
struct smack_known *skp = smk_of_current();
isp->security = skp;
@@ -2971,34 +2970,32 @@ static int smack_shm_alloc_security(struct kern_ipc_perm *shp)
}
/**
- * smack_shm_free_security - Clear the security blob for shm
- * @shp: the object
+ * smack_ipc_free_security - Clear the security blob for ipc
+ * @isp: the object
*
* Clears the blob pointer
*/
-static void smack_shm_free_security(struct kern_ipc_perm *shp)
+static void smack_ipc_free_security(struct kern_ipc_perm *isp)
{
- struct kern_ipc_perm *isp = shp;
-
isp->security = NULL;
}
/**
* smk_curacc_shm : check if current has access on shm
- * @shp : the object
+ * @isp : the object
* @access : access requested
*
* Returns 0 if current has the requested access, error code otherwise
*/
-static int smk_curacc_shm(struct kern_ipc_perm *shp, int access)
+static int smk_curacc_shm(struct kern_ipc_perm *isp, int access)
{
- struct smack_known *ssp = smack_of_shm(shp);
+ struct smack_known *ssp = smack_of_ipc(isp);
struct smk_audit_info ad;
int rc;
#ifdef CONFIG_AUDIT
smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_IPC);
- ad.a.u.ipc_id = shp->id;
+ ad.a.u.ipc_id = isp->id;
#endif
rc = smk_curacc(ssp, access, &ad);
rc = smk_bu_current("shm", ssp, access, rc);
@@ -3007,27 +3004,27 @@ static int smk_curacc_shm(struct kern_ipc_perm *shp, int access)
/**
* smack_shm_associate - Smack access check for shm
- * @shp: the object
+ * @isp: the object
* @shmflg: access requested
*
* Returns 0 if current has the requested access, error code otherwise
*/
-static int smack_shm_associate(struct kern_ipc_perm *shp, int shmflg)
+static int smack_shm_associate(struct kern_ipc_perm *isp, int shmflg)
{
int may;
may = smack_flags_to_may(shmflg);
- return smk_curacc_shm(shp, may);
+ return smk_curacc_shm(isp, may);
}
/**
* smack_shm_shmctl - Smack access check for shm
- * @shp: the object
+ * @isp: the object
* @cmd: what it wants to do
*
* Returns 0 if current has the requested access, error code otherwise
*/
-static int smack_shm_shmctl(struct kern_ipc_perm *shp, int cmd)
+static int smack_shm_shmctl(struct kern_ipc_perm *isp, int cmd)
{
int may;
@@ -3051,81 +3048,42 @@ static int smack_shm_shmctl(struct kern_ipc_perm *shp, int cmd)
default:
return -EINVAL;
}
- return smk_curacc_shm(shp, may);
+ return smk_curacc_shm(isp, may);
}
/**
* smack_shm_shmat - Smack access for shmat
- * @shp: the object
+ * @isp: the object
* @shmaddr: unused
* @shmflg: access requested
*
* Returns 0 if current has the requested access, error code otherwise
*/
-static int smack_shm_shmat(struct kern_ipc_perm *shp, char __user *shmaddr,
+static int smack_shm_shmat(struct kern_ipc_perm *ipc, char __user *shmaddr,
int shmflg)
{
int may;
may = smack_flags_to_may(shmflg);
- return smk_curacc_shm(shp, may);
-}
-
-/**
- * smack_of_sem - the smack pointer for the sem
- * @sma: the object
- *
- * Returns a pointer to the smack value
- */
-static struct smack_known *smack_of_sem(struct kern_ipc_perm *sma)
-{
- return (struct smack_known *)sma->security;
-}
-
-/**
- * smack_sem_alloc_security - Set the security blob for sem
- * @sma: the object
- *
- * Returns 0
- */
-static int smack_sem_alloc_security(struct kern_ipc_perm *sma)
-{
- struct kern_ipc_perm *isp = sma;
- struct smack_known *skp = smk_of_current();
-
- isp->security = skp;
- return 0;
-}
-
-/**
- * smack_sem_free_security - Clear the security blob for sem
- * @sma: the object
- *
- * Clears the blob pointer
- */
-static void smack_sem_free_security(struct kern_ipc_perm *sma)
-{
- struct kern_ipc_perm *isp = sma;
-
- isp->security = NULL;
+ return smk_curacc_shm(ipc, may);
}
/**
* smk_curacc_sem : check if current has access on sem
- * @sma : the object
+ * @isp : the object
* @access : access requested
*
* Returns 0 if current has the requested access, error code otherwise
*/
-static int smk_curacc_sem(struct kern_ipc_perm *sma, int access)
+static int smk_curacc_sem(struct kern_ipc_perm *isp, int access)
{
- struct smack_known *ssp = smack_of_sem(sma);
+ struct smack_known *ssp = smack_of_ipc(isp);
struct smk_audit_info ad;
int rc;
#ifdef CONFIG_AUDIT
smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_IPC);
- ad.a.u.ipc_id = sma->id;
+ ad.a.u.ipc_id = isp->id;
#endif
rc = smk_curacc(ssp, access, &ad);
rc = smk_bu_current("sem", ssp, access, rc);
@@ -3134,27 +3092,27 @@ static int smk_curacc_sem(struct kern_ipc_perm *sma, int access)
/**
* smack_sem_associate - Smack access check for sem
- * @sma: the object
+ * @isp: the object
* @semflg: access requested
*
* Returns 0 if current has the requested access, error code otherwise
*/
-static int smack_sem_associate(struct kern_ipc_perm *sma, int semflg)
+static int smack_sem_associate(struct kern_ipc_perm *isp, int semflg)
{
int may;
may = smack_flags_to_may(semflg);
- return smk_curacc_sem(sma, may);
+ return smk_curacc_sem(isp, may);
}
/**
* smack_sem_shmctl - Smack access check for sem
- * @sma: the object
+ * @isp: the object
* @cmd: what it wants to do
*
* Returns 0 if current has the requested access, error code otherwise
*/
-static int smack_sem_semctl(struct kern_ipc_perm *sma, int cmd)
+static int smack_sem_semctl(struct kern_ipc_perm *isp, int cmd)
{
int may;
@@ -3184,12 +3142,12 @@ static int smack_sem_semctl(struct kern_ipc_perm *sma, int cmd)
return -EINVAL;
}
- return smk_curacc_sem(sma, may);
+ return smk_curacc_sem(isp, may);
}
/**
* smack_sem_semop - Smack checks of semaphore operations
- * @sma: the object
+ * @isp: the object
* @sops: unused
* @nsops: unused
* @alter: unused
@@ -3198,67 +3156,28 @@ static int smack_sem_semctl(struct kern_ipc_perm *sma, int cmd)
*
* Returns 0 if access is allowed, error code otherwise
*/
-static int smack_sem_semop(struct kern_ipc_perm *sma, struct sembuf *sops,
+static int smack_sem_semop(struct kern_ipc_perm *isp, struct sembuf *sops,
unsigned nsops, int alter)
{
- return smk_curacc_sem(sma, MAY_READWRITE);
-}
-
-/**
- * smack_msg_alloc_security - Set the security blob for msg
- * @msq: the object
- *
- * Returns 0
- */
-static int smack_msg_queue_alloc_security(struct kern_ipc_perm *msq)
-{
- struct kern_ipc_perm *kisp = msq;
- struct smack_known *skp = smk_of_current();
-
- kisp->security = skp;
- return 0;
-}
-
-/**
- * smack_msg_free_security - Clear the security blob for msg
- * @msq: the object
- *
- * Clears the blob pointer
- */
-static void smack_msg_queue_free_security(struct kern_ipc_perm *msq)
-{
- struct kern_ipc_perm *kisp = msq;
-
- kisp->security = NULL;
-}
-
-/**
- * smack_of_msq - the smack pointer for the msq
- * @msq: the object
- *
- * Returns a pointer to the smack label entry
- */
-static struct smack_known *smack_of_msq(struct kern_ipc_perm *msq)
-{
- return (struct smack_known *)msq->security;
+ return smk_curacc_sem(isp, MAY_READWRITE);
}
/**
* smk_curacc_msq : helper to check if current has access on msq
- * @msq : the msq
+ * @isp : the msq
* @access : access requested
*
* return 0 if current has access, error otherwise
*/
-static int smk_curacc_msq(struct kern_ipc_perm *msq, int access)
+static int smk_curacc_msq(struct kern_ipc_perm *isp, int access)
{
- struct smack_known *msp = smack_of_msq(msq);
+ struct smack_known *msp = smack_of_ipc(isp);
struct smk_audit_info ad;
int rc;
#ifdef CONFIG_AUDIT
smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_IPC);
- ad.a.u.ipc_id = msq->id;
+ ad.a.u.ipc_id = isp->id;
#endif
rc = smk_curacc(msp, access, &ad);
rc = smk_bu_current("msq", msp, access, rc);
@@ -3267,27 +3186,27 @@ static int smk_curacc_msq(struct kern_ipc_perm *msq, int access)
/**
* smack_msg_queue_associate - Smack access check for msg_queue
- * @msq: the object
+ * @isp: the object
* @msqflg: access requested
*
* Returns 0 if current has the requested access, error code otherwise
*/
-static int smack_msg_queue_associate(struct kern_ipc_perm *msq, int msqflg)
+static int smack_msg_queue_associate(struct kern_ipc_perm *isp, int msqflg)
{
int may;
may = smack_flags_to_may(msqflg);
- return smk_curacc_msq(msq, may);
+ return smk_curacc_msq(isp, may);
}
/**
* smack_msg_queue_msgctl - Smack access check for msg_queue
- * @msq: the object
+ * @isp: the object
* @cmd: what it wants to do
*
* Returns 0 if current has the requested access, error code otherwise
*/
-static int smack_msg_queue_msgctl(struct kern_ipc_perm *msq, int cmd)
+static int smack_msg_queue_msgctl(struct kern_ipc_perm *isp, int cmd)
{
int may;
@@ -3310,29 +3229,29 @@ static int smack_msg_queue_msgctl(struct kern_ipc_perm *msq, int cmd)
return -EINVAL;
}
- return smk_curacc_msq(msq, may);
+ return smk_curacc_msq(isp, may);
}
/**
* smack_msg_queue_msgsnd - Smack access check for msg_queue
- * @msq: the object
+ * @isp: the object
* @msg: unused
* @msqflg: access requested
*
* Returns 0 if current has the requested access, error code otherwise
*/
-static int smack_msg_queue_msgsnd(struct kern_ipc_perm *msq, struct msg_msg *msg,
+static int smack_msg_queue_msgsnd(struct kern_ipc_perm *isp, struct msg_msg *msg,
int msqflg)
{
int may;
may = smack_flags_to_may(msqflg);
- return smk_curacc_msq(msq, may);
+ return smk_curacc_msq(isp, may);
}
/**
* smack_msg_queue_msgsnd - Smack access check for msg_queue
- * @msq: the object
+ * @isp: the object
* @msg: unused
* @target: unused
* @type: unused
@@ -3340,10 +3259,10 @@ static int smack_msg_queue_msgsnd(struct kern_ipc_perm *msq, struct msg_msg *msg
*
* Returns 0 if current has read and write access, error code otherwise
*/
-static int smack_msg_queue_msgrcv(struct kern_ipc_perm *msq, struct msg_msg *msg,
+static int smack_msg_queue_msgrcv(struct kern_ipc_perm *isp, struct msg_msg *msg,
struct task_struct *target, long type, int mode)
{
- return smk_curacc_msq(msq, MAY_READWRITE);
+ return smk_curacc_msq(isp, MAY_READWRITE);
}
/**
@@ -4756,21 +4675,21 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(msg_msg_alloc_security, smack_msg_msg_alloc_security),
LSM_HOOK_INIT(msg_msg_free_security, smack_msg_msg_free_security),
- LSM_HOOK_INIT(msg_queue_alloc_security, smack_msg_queue_alloc_security),
- LSM_HOOK_INIT(msg_queue_free_security, smack_msg_queue_free_security),
+ LSM_HOOK_INIT(msg_queue_alloc_security, smack_ipc_alloc_security),
+ LSM_HOOK_INIT(msg_queue_free_security, smack_ipc_free_security),
LSM_HOOK_INIT(msg_queue_associate, smack_msg_queue_associate),
LSM_HOOK_INIT(msg_queue_msgctl, smack_msg_queue_msgctl),
LSM_HOOK_INIT(msg_queue_msgsnd, smack_msg_queue_msgsnd),
LSM_HOOK_INIT(msg_queue_msgrcv, smack_msg_queue_msgrcv),
- LSM_HOOK_INIT(shm_alloc_security, smack_shm_alloc_security),
- LSM_HOOK_INIT(shm_free_security, smack_shm_free_security),
+ LSM_HOOK_INIT(shm_alloc_security, smack_ipc_alloc_security),
+ LSM_HOOK_INIT(shm_free_security, smack_ipc_free_security),
LSM_HOOK_INIT(shm_associate, smack_shm_associate),
LSM_HOOK_INIT(shm_shmctl, smack_shm_shmctl),
LSM_HOOK_INIT(shm_shmat, smack_shm_shmat),
- LSM_HOOK_INIT(sem_alloc_security, smack_sem_alloc_security),
- LSM_HOOK_INIT(sem_free_security, smack_sem_free_security),
+ LSM_HOOK_INIT(sem_alloc_security, smack_ipc_alloc_security),
+ LSM_HOOK_INIT(sem_free_security, smack_ipc_free_security),
LSM_HOOK_INIT(sem_associate, smack_sem_associate),
LSM_HOOK_INIT(sem_semctl, smack_sem_semctl),
LSM_HOOK_INIT(sem_semop, smack_sem_semop),
--
2.14.1
^ permalink raw reply related
* [REVIEW][PATCH 12/11] ipc: Directly call the security hook in ipc_ops.associate
From: Eric W. Biederman @ 2018-03-24 5:40 UTC (permalink / raw)
To: Linux Containers
Cc: jannh-hpIqsD4AKlfQT0dZR+AlfA, khlebnikov-XoJtRXgx1JseBXzfvpsJ4g,
linux-api-u79uwXL29TY76Z2rM5mHXA,
serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA, oleg-H+wXaHxf7aLQT0dZR+AlfA,
esyr-H+wXaHxf7aLQT0dZR+AlfA, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
luto-DgEjT+Ai2ygdnm+yROfE0A,
prakash.sangappa-QHcLZuEGTsvQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Nagarathnam Muthusamy,
Pavel Emelyanov
In-Reply-To: <87vadmobdw.fsf_-_-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
After the last round of cleanups the shm, sem, and msg associate
operations just became trivial wrappers around the appropriate security
method. Simplify things further by just calling the security method
directly.
Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
ipc/msg.c | 10 +---------
ipc/sem.c | 10 +---------
ipc/shm.c | 10 +---------
3 files changed, 3 insertions(+), 27 deletions(-)
diff --git a/ipc/msg.c b/ipc/msg.c
index 825ad585a6ff..d667dd8e97ab 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -272,20 +272,12 @@ static void freeque(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp)
ipc_rcu_putref(&msq->q_perm, msg_rcu_free);
}
-/*
- * Called with msg_ids.rwsem and ipcp locked.
- */
-static inline int msg_security(struct kern_ipc_perm *ipcp, int msgflg)
-{
- return security_msg_queue_associate(ipcp, msgflg);
-}
-
SYSCALL_DEFINE2(msgget, key_t, key, int, msgflg)
{
struct ipc_namespace *ns;
static const struct ipc_ops msg_ops = {
.getnew = newque,
- .associate = msg_security,
+ .associate = security_msg_queue_associate,
};
struct ipc_params msg_params;
diff --git a/ipc/sem.c b/ipc/sem.c
index 47b263960524..09d54af076a4 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -564,14 +564,6 @@ static int newary(struct ipc_namespace *ns, struct ipc_params *params)
}
-/*
- * Called with sem_ids.rwsem and ipcp locked.
- */
-static inline int sem_security(struct kern_ipc_perm *ipcp, int semflg)
-{
- return security_sem_associate(ipcp, semflg);
-}
-
/*
* Called with sem_ids.rwsem and ipcp locked.
*/
@@ -592,7 +584,7 @@ SYSCALL_DEFINE3(semget, key_t, key, int, nsems, int, semflg)
struct ipc_namespace *ns;
static const struct ipc_ops sem_ops = {
.getnew = newary,
- .associate = sem_security,
+ .associate = security_sem_associate,
.more_checks = sem_more_checks,
};
struct ipc_params sem_params;
diff --git a/ipc/shm.c b/ipc/shm.c
index 932b7e411c6c..018db3d0e70e 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -656,14 +656,6 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
return error;
}
-/*
- * Called with shm_ids.rwsem and ipcp locked.
- */
-static inline int shm_security(struct kern_ipc_perm *ipcp, int shmflg)
-{
- return security_shm_associate(ipcp, shmflg);
-}
-
/*
* Called with shm_ids.rwsem and ipcp locked.
*/
@@ -684,7 +676,7 @@ SYSCALL_DEFINE3(shmget, key_t, key, size_t, size, int, shmflg)
struct ipc_namespace *ns;
static const struct ipc_ops shm_ops = {
.getnew = newseg,
- .associate = shm_security,
+ .associate = security_shm_associate,
.more_checks = shm_more_checks,
};
struct ipc_params shm_params;
--
2.14.1
^ permalink raw reply related
* Re: [REVIEW][PATCH 03/11] msg/security: Pass kern_ipc_perm not msg_queue into the msg_queue security hooks
From: Eric W. Biederman @ 2018-03-24 5:37 UTC (permalink / raw)
To: Casey Schaufler
Cc: esyr-H+wXaHxf7aLQT0dZR+AlfA, jannh-hpIqsD4AKlfQT0dZR+AlfA,
khlebnikov-XoJtRXgx1JseBXzfvpsJ4g,
linux-api-u79uwXL29TY76Z2rM5mHXA, Linux Containers,
serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
prakash.sangappa-QHcLZuEGTsvQT0dZR+AlfA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
luto-DgEjT+Ai2ygdnm+yROfE0A, oleg-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Nagarathnam Muthusamy,
Pavel Emelyanov
In-Reply-To: <bb73b0ea-bcda-a996-8f14-48d9dd1b0940-iSGtlc1asvQWG2LlvL+J4A@public.gmane.org>
Casey Schaufler <casey-iSGtlc1asvQWG2LlvL+J4A@public.gmane.org> writes:
> On 3/23/2018 12:16 PM, Eric W. Biederman wrote:
>> All of the implementations of security hooks that take msg_queue only
>> access q_perm the struct kern_ipc_perm member. This means the
>> dependencies of the msg_queue security hooks can be simplified by
>> passing the kern_ipc_perm member of msg_queue.
>>
>> Making this change will allow struct msg_queue to become private to
>> ipc/msg.c.
>>
>> Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
>> ---
>> include/linux/lsm_hooks.h | 12 ++++++------
>> include/linux/security.h | 25 ++++++++++++-------------
>> ipc/msg.c | 18 ++++++++----------
>> security/security.c | 12 ++++++------
>> security/selinux/hooks.c | 36 ++++++++++++++++++------------------
>> security/smack/smack_lsm.c | 24 ++++++++++++------------
>
> Can I reference the comments I made in PATCH 01 of this set
> regarding the Smack changes? The problem in all of your changes
> is the same. You aren't preserving the naming conventions, and
> you've left in some code that is just silly.
Being silly like that is actually important to make a sweeping patch
like that boring and trivial to show that it is correct. Anything
that is not a rule based transformation is much more likely to hide
a bug. So for the push down of the type change I think it was the right
way to go.
That said I am happy to add a clean up patch that makes the obvious
cleanups and simplifications to smack_lsm.c.
Eric
^ permalink raw reply
* Re: [REVIEW][PATCH 03/11] msg/security: Pass kern_ipc_perm not msg_queue into the msg_queue security hooks
From: Casey Schaufler @ 2018-03-23 21:55 UTC (permalink / raw)
To: Eric W. Biederman, Linux Containers
Cc: linux-kernel, linux-api, khlebnikov, prakash.sangappa, luto, akpm,
oleg, serge.hallyn, esyr, jannh, linux-security-module,
Pavel Emelyanov, Nagarathnam Muthusamy
In-Reply-To: <20180323191614.32489-3-ebiederm@xmission.com>
On 3/23/2018 12:16 PM, Eric W. Biederman wrote:
> All of the implementations of security hooks that take msg_queue only
> access q_perm the struct kern_ipc_perm member. This means the
> dependencies of the msg_queue security hooks can be simplified by
> passing the kern_ipc_perm member of msg_queue.
>
> Making this change will allow struct msg_queue to become private to
> ipc/msg.c.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
> include/linux/lsm_hooks.h | 12 ++++++------
> include/linux/security.h | 25 ++++++++++++-------------
> ipc/msg.c | 18 ++++++++----------
> security/security.c | 12 ++++++------
> security/selinux/hooks.c | 36 ++++++++++++++++++------------------
> security/smack/smack_lsm.c | 24 ++++++++++++------------
Can I reference the comments I made in PATCH 01 of this set
regarding the Smack changes? The problem in all of your changes
is the same. You aren't preserving the naming conventions, and
you've left in some code that is just silly.
> 6 files changed, 62 insertions(+), 65 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index cac7a8082c43..bde167fa2c51 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1575,13 +1575,13 @@ union security_list_options {
> int (*msg_msg_alloc_security)(struct msg_msg *msg);
> void (*msg_msg_free_security)(struct msg_msg *msg);
>
> - int (*msg_queue_alloc_security)(struct msg_queue *msq);
> - void (*msg_queue_free_security)(struct msg_queue *msq);
> - int (*msg_queue_associate)(struct msg_queue *msq, int msqflg);
> - int (*msg_queue_msgctl)(struct msg_queue *msq, int cmd);
> - int (*msg_queue_msgsnd)(struct msg_queue *msq, struct msg_msg *msg,
> + int (*msg_queue_alloc_security)(struct kern_ipc_perm *msq);
> + void (*msg_queue_free_security)(struct kern_ipc_perm *msq);
> + int (*msg_queue_associate)(struct kern_ipc_perm *msq, int msqflg);
> + int (*msg_queue_msgctl)(struct kern_ipc_perm *msq, int cmd);
> + int (*msg_queue_msgsnd)(struct kern_ipc_perm *msq, struct msg_msg *msg,
> int msqflg);
> - int (*msg_queue_msgrcv)(struct msg_queue *msq, struct msg_msg *msg,
> + int (*msg_queue_msgrcv)(struct kern_ipc_perm *msq, struct msg_msg *msg,
> struct task_struct *target, long type,
> int mode);
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index f390755808ea..128e1e4a5346 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -50,7 +50,6 @@ struct iattr;
> struct fown_struct;
> struct file_operations;
> struct msg_msg;
> -struct msg_queue;
> struct xattr;
> struct xfrm_sec_ctx;
> struct mm_struct;
> @@ -353,13 +352,13 @@ int security_ipc_permission(struct kern_ipc_perm *ipcp, short flag);
> void security_ipc_getsecid(struct kern_ipc_perm *ipcp, u32 *secid);
> int security_msg_msg_alloc(struct msg_msg *msg);
> void security_msg_msg_free(struct msg_msg *msg);
> -int security_msg_queue_alloc(struct msg_queue *msq);
> -void security_msg_queue_free(struct msg_queue *msq);
> -int security_msg_queue_associate(struct msg_queue *msq, int msqflg);
> -int security_msg_queue_msgctl(struct msg_queue *msq, int cmd);
> -int security_msg_queue_msgsnd(struct msg_queue *msq,
> +int security_msg_queue_alloc(struct kern_ipc_perm *msq);
> +void security_msg_queue_free(struct kern_ipc_perm *msq);
> +int security_msg_queue_associate(struct kern_ipc_perm *msq, int msqflg);
> +int security_msg_queue_msgctl(struct kern_ipc_perm *msq, int cmd);
> +int security_msg_queue_msgsnd(struct kern_ipc_perm *msq,
> struct msg_msg *msg, int msqflg);
> -int security_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg *msg,
> +int security_msg_queue_msgrcv(struct kern_ipc_perm *msq, struct msg_msg *msg,
> struct task_struct *target, long type, int mode);
> int security_shm_alloc(struct kern_ipc_perm *shp);
> void security_shm_free(struct kern_ipc_perm *shp);
> @@ -1043,32 +1042,32 @@ static inline int security_msg_msg_alloc(struct msg_msg *msg)
> static inline void security_msg_msg_free(struct msg_msg *msg)
> { }
>
> -static inline int security_msg_queue_alloc(struct msg_queue *msq)
> +static inline int security_msg_queue_alloc(struct kern_ipc_perm *msq)
> {
> return 0;
> }
>
> -static inline void security_msg_queue_free(struct msg_queue *msq)
> +static inline void security_msg_queue_free(struct kern_ipc_perm *msq)
> { }
>
> -static inline int security_msg_queue_associate(struct msg_queue *msq,
> +static inline int security_msg_queue_associate(struct kern_ipc_perm *msq,
> int msqflg)
> {
> return 0;
> }
>
> -static inline int security_msg_queue_msgctl(struct msg_queue *msq, int cmd)
> +static inline int security_msg_queue_msgctl(struct kern_ipc_perm *msq, int cmd)
> {
> return 0;
> }
>
> -static inline int security_msg_queue_msgsnd(struct msg_queue *msq,
> +static inline int security_msg_queue_msgsnd(struct kern_ipc_perm *msq,
> struct msg_msg *msg, int msqflg)
> {
> return 0;
> }
>
> -static inline int security_msg_queue_msgrcv(struct msg_queue *msq,
> +static inline int security_msg_queue_msgrcv(struct kern_ipc_perm *msq,
> struct msg_msg *msg,
> struct task_struct *target,
> long type, int mode)
> diff --git a/ipc/msg.c b/ipc/msg.c
> index 0dcc6699dc53..cdfab0825fce 100644
> --- a/ipc/msg.c
> +++ b/ipc/msg.c
> @@ -101,7 +101,7 @@ static void msg_rcu_free(struct rcu_head *head)
> struct kern_ipc_perm *p = container_of(head, struct kern_ipc_perm, rcu);
> struct msg_queue *msq = container_of(p, struct msg_queue, q_perm);
>
> - security_msg_queue_free(msq);
> + security_msg_queue_free(&msq->q_perm);
> kvfree(msq);
> }
>
> @@ -127,7 +127,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
> msq->q_perm.key = key;
>
> msq->q_perm.security = NULL;
> - retval = security_msg_queue_alloc(msq);
> + retval = security_msg_queue_alloc(&msq->q_perm);
> if (retval) {
> kvfree(msq);
> return retval;
> @@ -258,9 +258,7 @@ static void freeque(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp)
> */
> static inline int msg_security(struct kern_ipc_perm *ipcp, int msgflg)
> {
> - struct msg_queue *msq = container_of(ipcp, struct msg_queue, q_perm);
> -
> - return security_msg_queue_associate(msq, msgflg);
> + return security_msg_queue_associate(ipcp, msgflg);
> }
>
> SYSCALL_DEFINE2(msgget, key_t, key, int, msgflg)
> @@ -380,7 +378,7 @@ static int msgctl_down(struct ipc_namespace *ns, int msqid, int cmd,
>
> msq = container_of(ipcp, struct msg_queue, q_perm);
>
> - err = security_msg_queue_msgctl(msq, cmd);
> + err = security_msg_queue_msgctl(&msq->q_perm, cmd);
> if (err)
> goto out_unlock1;
>
> @@ -502,7 +500,7 @@ static int msgctl_stat(struct ipc_namespace *ns, int msqid,
> if (ipcperms(ns, &msq->q_perm, S_IRUGO))
> goto out_unlock;
>
> - err = security_msg_queue_msgctl(msq, cmd);
> + err = security_msg_queue_msgctl(&msq->q_perm, cmd);
> if (err)
> goto out_unlock;
>
> @@ -718,7 +716,7 @@ static inline int pipelined_send(struct msg_queue *msq, struct msg_msg *msg,
>
> list_for_each_entry_safe(msr, t, &msq->q_receivers, r_list) {
> if (testmsg(msg, msr->r_msgtype, msr->r_mode) &&
> - !security_msg_queue_msgrcv(msq, msg, msr->r_tsk,
> + !security_msg_queue_msgrcv(&msq->q_perm, msg, msr->r_tsk,
> msr->r_msgtype, msr->r_mode)) {
>
> list_del(&msr->r_list);
> @@ -784,7 +782,7 @@ static long do_msgsnd(int msqid, long mtype, void __user *mtext,
> goto out_unlock0;
> }
>
> - err = security_msg_queue_msgsnd(msq, msg, msgflg);
> + err = security_msg_queue_msgsnd(&msq->q_perm, msg, msgflg);
> if (err)
> goto out_unlock0;
>
> @@ -960,7 +958,7 @@ static struct msg_msg *find_msg(struct msg_queue *msq, long *msgtyp, int mode)
>
> list_for_each_entry(msg, &msq->q_messages, m_list) {
> if (testmsg(msg, *msgtyp, mode) &&
> - !security_msg_queue_msgrcv(msq, msg, current,
> + !security_msg_queue_msgrcv(&msq->q_perm, msg, current,
> *msgtyp, mode)) {
> if (mode == SEARCH_LESSEQUAL && msg->m_type != 1) {
> *msgtyp = msg->m_type - 1;
> diff --git a/security/security.c b/security/security.c
> index 77b69bd6f234..02d734e69955 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1163,33 +1163,33 @@ void security_msg_msg_free(struct msg_msg *msg)
> call_void_hook(msg_msg_free_security, msg);
> }
>
> -int security_msg_queue_alloc(struct msg_queue *msq)
> +int security_msg_queue_alloc(struct kern_ipc_perm *msq)
> {
> return call_int_hook(msg_queue_alloc_security, 0, msq);
> }
>
> -void security_msg_queue_free(struct msg_queue *msq)
> +void security_msg_queue_free(struct kern_ipc_perm *msq)
> {
> call_void_hook(msg_queue_free_security, msq);
> }
>
> -int security_msg_queue_associate(struct msg_queue *msq, int msqflg)
> +int security_msg_queue_associate(struct kern_ipc_perm *msq, int msqflg)
> {
> return call_int_hook(msg_queue_associate, 0, msq, msqflg);
> }
>
> -int security_msg_queue_msgctl(struct msg_queue *msq, int cmd)
> +int security_msg_queue_msgctl(struct kern_ipc_perm *msq, int cmd)
> {
> return call_int_hook(msg_queue_msgctl, 0, msq, cmd);
> }
>
> -int security_msg_queue_msgsnd(struct msg_queue *msq,
> +int security_msg_queue_msgsnd(struct kern_ipc_perm *msq,
> struct msg_msg *msg, int msqflg)
> {
> return call_int_hook(msg_queue_msgsnd, 0, msq, msg, msqflg);
> }
>
> -int security_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg *msg,
> +int security_msg_queue_msgrcv(struct kern_ipc_perm *msq, struct msg_msg *msg,
> struct task_struct *target, long type, int mode)
> {
> return call_int_hook(msg_queue_msgrcv, 0, msq, msg, target, type, mode);
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 14f9e6c08273..925e546b5a87 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -5532,52 +5532,52 @@ static void selinux_msg_msg_free_security(struct msg_msg *msg)
> }
>
> /* message queue security operations */
> -static int selinux_msg_queue_alloc_security(struct msg_queue *msq)
> +static int selinux_msg_queue_alloc_security(struct kern_ipc_perm *msq)
> {
> struct ipc_security_struct *isec;
> struct common_audit_data ad;
> u32 sid = current_sid();
> int rc;
>
> - rc = ipc_alloc_security(&msq->q_perm, SECCLASS_MSGQ);
> + rc = ipc_alloc_security(msq, SECCLASS_MSGQ);
> if (rc)
> return rc;
>
> - isec = msq->q_perm.security;
> + isec = msq->security;
>
> ad.type = LSM_AUDIT_DATA_IPC;
> - ad.u.ipc_id = msq->q_perm.key;
> + ad.u.ipc_id = msq->key;
>
> rc = avc_has_perm(sid, isec->sid, SECCLASS_MSGQ,
> MSGQ__CREATE, &ad);
> if (rc) {
> - ipc_free_security(&msq->q_perm);
> + ipc_free_security(msq);
> return rc;
> }
> return 0;
> }
>
> -static void selinux_msg_queue_free_security(struct msg_queue *msq)
> +static void selinux_msg_queue_free_security(struct kern_ipc_perm *msq)
> {
> - ipc_free_security(&msq->q_perm);
> + ipc_free_security(msq);
> }
>
> -static int selinux_msg_queue_associate(struct msg_queue *msq, int msqflg)
> +static int selinux_msg_queue_associate(struct kern_ipc_perm *msq, int msqflg)
> {
> struct ipc_security_struct *isec;
> struct common_audit_data ad;
> u32 sid = current_sid();
>
> - isec = msq->q_perm.security;
> + isec = msq->security;
>
> ad.type = LSM_AUDIT_DATA_IPC;
> - ad.u.ipc_id = msq->q_perm.key;
> + ad.u.ipc_id = msq->key;
>
> return avc_has_perm(sid, isec->sid, SECCLASS_MSGQ,
> MSGQ__ASSOCIATE, &ad);
> }
>
> -static int selinux_msg_queue_msgctl(struct msg_queue *msq, int cmd)
> +static int selinux_msg_queue_msgctl(struct kern_ipc_perm *msq, int cmd)
> {
> int err;
> int perms;
> @@ -5602,11 +5602,11 @@ static int selinux_msg_queue_msgctl(struct msg_queue *msq, int cmd)
> return 0;
> }
>
> - err = ipc_has_perm(&msq->q_perm, perms);
> + err = ipc_has_perm(msq, perms);
> return err;
> }
>
> -static int selinux_msg_queue_msgsnd(struct msg_queue *msq, struct msg_msg *msg, int msqflg)
> +static int selinux_msg_queue_msgsnd(struct kern_ipc_perm *msq, struct msg_msg *msg, int msqflg)
> {
> struct ipc_security_struct *isec;
> struct msg_security_struct *msec;
> @@ -5614,7 +5614,7 @@ static int selinux_msg_queue_msgsnd(struct msg_queue *msq, struct msg_msg *msg,
> u32 sid = current_sid();
> int rc;
>
> - isec = msq->q_perm.security;
> + isec = msq->security;
> msec = msg->security;
>
> /*
> @@ -5632,7 +5632,7 @@ static int selinux_msg_queue_msgsnd(struct msg_queue *msq, struct msg_msg *msg,
> }
>
> ad.type = LSM_AUDIT_DATA_IPC;
> - ad.u.ipc_id = msq->q_perm.key;
> + ad.u.ipc_id = msq->key;
>
> /* Can this process write to the queue? */
> rc = avc_has_perm(sid, isec->sid, SECCLASS_MSGQ,
> @@ -5649,7 +5649,7 @@ static int selinux_msg_queue_msgsnd(struct msg_queue *msq, struct msg_msg *msg,
> return rc;
> }
>
> -static int selinux_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg *msg,
> +static int selinux_msg_queue_msgrcv(struct kern_ipc_perm *msq, struct msg_msg *msg,
> struct task_struct *target,
> long type, int mode)
> {
> @@ -5659,11 +5659,11 @@ static int selinux_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg *msg,
> u32 sid = task_sid(target);
> int rc;
>
> - isec = msq->q_perm.security;
> + isec = msq->security;
> msec = msg->security;
>
> ad.type = LSM_AUDIT_DATA_IPC;
> - ad.u.ipc_id = msq->q_perm.key;
> + ad.u.ipc_id = msq->key;
>
> rc = avc_has_perm(sid, isec->sid,
> SECCLASS_MSGQ, MSGQ__READ, &ad);
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index a3398c7f32c9..d960c2ea8d79 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -3210,9 +3210,9 @@ static int smack_sem_semop(struct kern_ipc_perm *sma, struct sembuf *sops,
> *
> * Returns 0
> */
> -static int smack_msg_queue_alloc_security(struct msg_queue *msq)
> +static int smack_msg_queue_alloc_security(struct kern_ipc_perm *msq)
> {
> - struct kern_ipc_perm *kisp = &msq->q_perm;
> + struct kern_ipc_perm *kisp = msq;
> struct smack_known *skp = smk_of_current();
>
> kisp->security = skp;
> @@ -3225,9 +3225,9 @@ static int smack_msg_queue_alloc_security(struct msg_queue *msq)
> *
> * Clears the blob pointer
> */
> -static void smack_msg_queue_free_security(struct msg_queue *msq)
> +static void smack_msg_queue_free_security(struct kern_ipc_perm *msq)
> {
> - struct kern_ipc_perm *kisp = &msq->q_perm;
> + struct kern_ipc_perm *kisp = msq;
>
> kisp->security = NULL;
> }
> @@ -3238,9 +3238,9 @@ static void smack_msg_queue_free_security(struct msg_queue *msq)
> *
> * Returns a pointer to the smack label entry
> */
> -static struct smack_known *smack_of_msq(struct msg_queue *msq)
> +static struct smack_known *smack_of_msq(struct kern_ipc_perm *msq)
> {
> - return (struct smack_known *)msq->q_perm.security;
> + return (struct smack_known *)msq->security;
> }
>
> /**
> @@ -3250,7 +3250,7 @@ static struct smack_known *smack_of_msq(struct msg_queue *msq)
> *
> * return 0 if current has access, error otherwise
> */
> -static int smk_curacc_msq(struct msg_queue *msq, int access)
> +static int smk_curacc_msq(struct kern_ipc_perm *msq, int access)
> {
> struct smack_known *msp = smack_of_msq(msq);
> struct smk_audit_info ad;
> @@ -3258,7 +3258,7 @@ static int smk_curacc_msq(struct msg_queue *msq, int access)
>
> #ifdef CONFIG_AUDIT
> smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_IPC);
> - ad.a.u.ipc_id = msq->q_perm.id;
> + ad.a.u.ipc_id = msq->id;
> #endif
> rc = smk_curacc(msp, access, &ad);
> rc = smk_bu_current("msq", msp, access, rc);
> @@ -3272,7 +3272,7 @@ static int smk_curacc_msq(struct msg_queue *msq, int access)
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smack_msg_queue_associate(struct msg_queue *msq, int msqflg)
> +static int smack_msg_queue_associate(struct kern_ipc_perm *msq, int msqflg)
> {
> int may;
>
> @@ -3287,7 +3287,7 @@ static int smack_msg_queue_associate(struct msg_queue *msq, int msqflg)
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smack_msg_queue_msgctl(struct msg_queue *msq, int cmd)
> +static int smack_msg_queue_msgctl(struct kern_ipc_perm *msq, int cmd)
> {
> int may;
>
> @@ -3321,7 +3321,7 @@ static int smack_msg_queue_msgctl(struct msg_queue *msq, int cmd)
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smack_msg_queue_msgsnd(struct msg_queue *msq, struct msg_msg *msg,
> +static int smack_msg_queue_msgsnd(struct kern_ipc_perm *msq, struct msg_msg *msg,
> int msqflg)
> {
> int may;
> @@ -3340,7 +3340,7 @@ static int smack_msg_queue_msgsnd(struct msg_queue *msq, struct msg_msg *msg,
> *
> * Returns 0 if current has read and write access, error code otherwise
> */
> -static int smack_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg *msg,
> +static int smack_msg_queue_msgrcv(struct kern_ipc_perm *msq, struct msg_msg *msg,
> struct task_struct *target, long type, int mode)
> {
> return smk_curacc_msq(msq, MAY_READWRITE);
^ permalink raw reply
* Re: [REVIEW][PATCH 02/11] shm/security: Pass kern_ipc_perm not shmid_kernel into the shm security hooks
From: Casey Schaufler @ 2018-03-23 21:54 UTC (permalink / raw)
To: Eric W. Biederman, Linux Containers
Cc: linux-kernel, linux-api, khlebnikov, prakash.sangappa, luto, akpm,
oleg, serge.hallyn, esyr, jannh, linux-security-module,
Pavel Emelyanov, Nagarathnam Muthusamy
In-Reply-To: <20180323191614.32489-2-ebiederm@xmission.com>
On 3/23/2018 12:16 PM, Eric W. Biederman wrote:
> All of the implementations of security hooks that take shmid_kernel only
> access shm_perm the struct kern_ipc_perm member. This means the
> dependencies of the shm security hooks can be simplified by passing
> the kern_ipc_perm member of shmid_kernel..
>
> Making this change will allow struct shmid_kernel to become private to ipc/shm.c.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
> include/linux/lsm_hooks.h | 10 +++++-----
> include/linux/security.h | 21 ++++++++++-----------
> ipc/shm.c | 17 +++++++----------
> security/security.c | 10 +++++-----
> security/selinux/hooks.c | 28 ++++++++++++++--------------
> security/smack/smack_lsm.c | 22 +++++++++++-----------
Can I reference the comments I made in PATCH 01 of this set
regarding the Smack changes? The problem in all of your changes
is the same. You aren't preserving the naming conventions, and
you've left in some code that is just silly.
> 6 files changed, 52 insertions(+), 56 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index e4a94863a88c..cac7a8082c43 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1585,11 +1585,11 @@ union security_list_options {
> struct task_struct *target, long type,
> int mode);
>
> - int (*shm_alloc_security)(struct shmid_kernel *shp);
> - void (*shm_free_security)(struct shmid_kernel *shp);
> - int (*shm_associate)(struct shmid_kernel *shp, int shmflg);
> - int (*shm_shmctl)(struct shmid_kernel *shp, int cmd);
> - int (*shm_shmat)(struct shmid_kernel *shp, char __user *shmaddr,
> + int (*shm_alloc_security)(struct kern_ipc_perm *shp);
> + void (*shm_free_security)(struct kern_ipc_perm *shp);
> + int (*shm_associate)(struct kern_ipc_perm *shp, int shmflg);
> + int (*shm_shmctl)(struct kern_ipc_perm *shp, int cmd);
> + int (*shm_shmat)(struct kern_ipc_perm *shp, char __user *shmaddr,
> int shmflg);
>
> int (*sem_alloc_security)(struct kern_ipc_perm *sma);
> diff --git a/include/linux/security.h b/include/linux/security.h
> index fa7adac4b99a..f390755808ea 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -49,7 +49,6 @@ struct qstr;
> struct iattr;
> struct fown_struct;
> struct file_operations;
> -struct shmid_kernel;
> struct msg_msg;
> struct msg_queue;
> struct xattr;
> @@ -362,11 +361,11 @@ int security_msg_queue_msgsnd(struct msg_queue *msq,
> struct msg_msg *msg, int msqflg);
> int security_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg *msg,
> struct task_struct *target, long type, int mode);
> -int security_shm_alloc(struct shmid_kernel *shp);
> -void security_shm_free(struct shmid_kernel *shp);
> -int security_shm_associate(struct shmid_kernel *shp, int shmflg);
> -int security_shm_shmctl(struct shmid_kernel *shp, int cmd);
> -int security_shm_shmat(struct shmid_kernel *shp, char __user *shmaddr, int shmflg);
> +int security_shm_alloc(struct kern_ipc_perm *shp);
> +void security_shm_free(struct kern_ipc_perm *shp);
> +int security_shm_associate(struct kern_ipc_perm *shp, int shmflg);
> +int security_shm_shmctl(struct kern_ipc_perm *shp, int cmd);
> +int security_shm_shmat(struct kern_ipc_perm *shp, char __user *shmaddr, int shmflg);
> int security_sem_alloc(struct kern_ipc_perm *sma);
> void security_sem_free(struct kern_ipc_perm *sma);
> int security_sem_associate(struct kern_ipc_perm *sma, int semflg);
> @@ -1077,26 +1076,26 @@ static inline int security_msg_queue_msgrcv(struct msg_queue *msq,
> return 0;
> }
>
> -static inline int security_shm_alloc(struct shmid_kernel *shp)
> +static inline int security_shm_alloc(struct kern_ipc_perm *shp)
> {
> return 0;
> }
>
> -static inline void security_shm_free(struct shmid_kernel *shp)
> +static inline void security_shm_free(struct kern_ipc_perm *shp)
> { }
>
> -static inline int security_shm_associate(struct shmid_kernel *shp,
> +static inline int security_shm_associate(struct kern_ipc_perm *shp,
> int shmflg)
> {
> return 0;
> }
>
> -static inline int security_shm_shmctl(struct shmid_kernel *shp, int cmd)
> +static inline int security_shm_shmctl(struct kern_ipc_perm *shp, int cmd)
> {
> return 0;
> }
>
> -static inline int security_shm_shmat(struct shmid_kernel *shp,
> +static inline int security_shm_shmat(struct kern_ipc_perm *shp,
> char __user *shmaddr, int shmflg)
> {
> return 0;
> diff --git a/ipc/shm.c b/ipc/shm.c
> index 4643865e9171..387a786e7be1 100644
> --- a/ipc/shm.c
> +++ b/ipc/shm.c
> @@ -181,7 +181,7 @@ static void shm_rcu_free(struct rcu_head *head)
> rcu);
> struct shmid_kernel *shp = container_of(ptr, struct shmid_kernel,
> shm_perm);
> - security_shm_free(shp);
> + security_shm_free(&shp->shm_perm);
> kvfree(shp);
> }
>
> @@ -554,7 +554,7 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
> shp->mlock_user = NULL;
>
> shp->shm_perm.security = NULL;
> - error = security_shm_alloc(shp);
> + error = security_shm_alloc(&shp->shm_perm);
> if (error) {
> kvfree(shp);
> return error;
> @@ -635,10 +635,7 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
> */
> static inline int shm_security(struct kern_ipc_perm *ipcp, int shmflg)
> {
> - struct shmid_kernel *shp;
> -
> - shp = container_of(ipcp, struct shmid_kernel, shm_perm);
> - return security_shm_associate(shp, shmflg);
> + return security_shm_associate(ipcp, shmflg);
> }
>
> /*
> @@ -835,7 +832,7 @@ static int shmctl_down(struct ipc_namespace *ns, int shmid, int cmd,
>
> shp = container_of(ipcp, struct shmid_kernel, shm_perm);
>
> - err = security_shm_shmctl(shp, cmd);
> + err = security_shm_shmctl(&shp->shm_perm, cmd);
> if (err)
> goto out_unlock1;
>
> @@ -934,7 +931,7 @@ static int shmctl_stat(struct ipc_namespace *ns, int shmid,
> if (ipcperms(ns, &shp->shm_perm, S_IRUGO))
> goto out_unlock;
>
> - err = security_shm_shmctl(shp, cmd);
> + err = security_shm_shmctl(&shp->shm_perm, cmd);
> if (err)
> goto out_unlock;
>
> @@ -978,7 +975,7 @@ static int shmctl_do_lock(struct ipc_namespace *ns, int shmid, int cmd)
> }
>
> audit_ipc_obj(&(shp->shm_perm));
> - err = security_shm_shmctl(shp, cmd);
> + err = security_shm_shmctl(&shp->shm_perm, cmd);
> if (err)
> goto out_unlock1;
>
> @@ -1348,7 +1345,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg,
> if (ipcperms(ns, &shp->shm_perm, acc_mode))
> goto out_unlock;
>
> - err = security_shm_shmat(shp, shmaddr, shmflg);
> + err = security_shm_shmat(&shp->shm_perm, shmaddr, shmflg);
> if (err)
> goto out_unlock;
>
> diff --git a/security/security.c b/security/security.c
> index d3b9aeb6b73b..77b69bd6f234 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1195,27 +1195,27 @@ int security_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg *msg,
> return call_int_hook(msg_queue_msgrcv, 0, msq, msg, target, type, mode);
> }
>
> -int security_shm_alloc(struct shmid_kernel *shp)
> +int security_shm_alloc(struct kern_ipc_perm *shp)
> {
> return call_int_hook(shm_alloc_security, 0, shp);
> }
>
> -void security_shm_free(struct shmid_kernel *shp)
> +void security_shm_free(struct kern_ipc_perm *shp)
> {
> call_void_hook(shm_free_security, shp);
> }
>
> -int security_shm_associate(struct shmid_kernel *shp, int shmflg)
> +int security_shm_associate(struct kern_ipc_perm *shp, int shmflg)
> {
> return call_int_hook(shm_associate, 0, shp, shmflg);
> }
>
> -int security_shm_shmctl(struct shmid_kernel *shp, int cmd)
> +int security_shm_shmctl(struct kern_ipc_perm *shp, int cmd)
> {
> return call_int_hook(shm_shmctl, 0, shp, cmd);
> }
>
> -int security_shm_shmat(struct shmid_kernel *shp, char __user *shmaddr, int shmflg)
> +int security_shm_shmat(struct kern_ipc_perm *shp, char __user *shmaddr, int shmflg)
> {
> return call_int_hook(shm_shmat, 0, shp, shmaddr, shmflg);
> }
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index cce994e9fc0a..14f9e6c08273 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -5674,53 +5674,53 @@ static int selinux_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg *msg,
> }
>
> /* Shared Memory security operations */
> -static int selinux_shm_alloc_security(struct shmid_kernel *shp)
> +static int selinux_shm_alloc_security(struct kern_ipc_perm *shp)
> {
> struct ipc_security_struct *isec;
> struct common_audit_data ad;
> u32 sid = current_sid();
> int rc;
>
> - rc = ipc_alloc_security(&shp->shm_perm, SECCLASS_SHM);
> + rc = ipc_alloc_security(shp, SECCLASS_SHM);
> if (rc)
> return rc;
>
> - isec = shp->shm_perm.security;
> + isec = shp->security;
>
> ad.type = LSM_AUDIT_DATA_IPC;
> - ad.u.ipc_id = shp->shm_perm.key;
> + ad.u.ipc_id = shp->key;
>
> rc = avc_has_perm(sid, isec->sid, SECCLASS_SHM,
> SHM__CREATE, &ad);
> if (rc) {
> - ipc_free_security(&shp->shm_perm);
> + ipc_free_security(shp);
> return rc;
> }
> return 0;
> }
>
> -static void selinux_shm_free_security(struct shmid_kernel *shp)
> +static void selinux_shm_free_security(struct kern_ipc_perm *shp)
> {
> - ipc_free_security(&shp->shm_perm);
> + ipc_free_security(shp);
> }
>
> -static int selinux_shm_associate(struct shmid_kernel *shp, int shmflg)
> +static int selinux_shm_associate(struct kern_ipc_perm *shp, int shmflg)
> {
> struct ipc_security_struct *isec;
> struct common_audit_data ad;
> u32 sid = current_sid();
>
> - isec = shp->shm_perm.security;
> + isec = shp->security;
>
> ad.type = LSM_AUDIT_DATA_IPC;
> - ad.u.ipc_id = shp->shm_perm.key;
> + ad.u.ipc_id = shp->key;
>
> return avc_has_perm(sid, isec->sid, SECCLASS_SHM,
> SHM__ASSOCIATE, &ad);
> }
>
> /* Note, at this point, shp is locked down */
> -static int selinux_shm_shmctl(struct shmid_kernel *shp, int cmd)
> +static int selinux_shm_shmctl(struct kern_ipc_perm *shp, int cmd)
> {
> int perms;
> int err;
> @@ -5749,11 +5749,11 @@ static int selinux_shm_shmctl(struct shmid_kernel *shp, int cmd)
> return 0;
> }
>
> - err = ipc_has_perm(&shp->shm_perm, perms);
> + err = ipc_has_perm(shp, perms);
> return err;
> }
>
> -static int selinux_shm_shmat(struct shmid_kernel *shp,
> +static int selinux_shm_shmat(struct kern_ipc_perm *shp,
> char __user *shmaddr, int shmflg)
> {
> u32 perms;
> @@ -5763,7 +5763,7 @@ static int selinux_shm_shmat(struct shmid_kernel *shp,
> else
> perms = SHM__READ | SHM__WRITE;
>
> - return ipc_has_perm(&shp->shm_perm, perms);
> + return ipc_has_perm(shp, perms);
> }
>
> /* Semaphore security operations */
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 0402b8c1aec1..a3398c7f32c9 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -2950,9 +2950,9 @@ static void smack_msg_msg_free_security(struct msg_msg *msg)
> *
> * Returns a pointer to the smack value
> */
> -static struct smack_known *smack_of_shm(struct shmid_kernel *shp)
> +static struct smack_known *smack_of_shm(struct kern_ipc_perm *shp)
> {
> - return (struct smack_known *)shp->shm_perm.security;
> + return (struct smack_known *)shp->security;
> }
>
> /**
> @@ -2961,9 +2961,9 @@ static struct smack_known *smack_of_shm(struct shmid_kernel *shp)
> *
> * Returns 0
> */
> -static int smack_shm_alloc_security(struct shmid_kernel *shp)
> +static int smack_shm_alloc_security(struct kern_ipc_perm *shp)
> {
> - struct kern_ipc_perm *isp = &shp->shm_perm;
> + struct kern_ipc_perm *isp = shp;
> struct smack_known *skp = smk_of_current();
>
> isp->security = skp;
> @@ -2976,9 +2976,9 @@ static int smack_shm_alloc_security(struct shmid_kernel *shp)
> *
> * Clears the blob pointer
> */
> -static void smack_shm_free_security(struct shmid_kernel *shp)
> +static void smack_shm_free_security(struct kern_ipc_perm *shp)
> {
> - struct kern_ipc_perm *isp = &shp->shm_perm;
> + struct kern_ipc_perm *isp = shp;
>
> isp->security = NULL;
> }
> @@ -2990,7 +2990,7 @@ static void smack_shm_free_security(struct shmid_kernel *shp)
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smk_curacc_shm(struct shmid_kernel *shp, int access)
> +static int smk_curacc_shm(struct kern_ipc_perm *shp, int access)
> {
> struct smack_known *ssp = smack_of_shm(shp);
> struct smk_audit_info ad;
> @@ -2998,7 +2998,7 @@ static int smk_curacc_shm(struct shmid_kernel *shp, int access)
>
> #ifdef CONFIG_AUDIT
> smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_IPC);
> - ad.a.u.ipc_id = shp->shm_perm.id;
> + ad.a.u.ipc_id = shp->id;
> #endif
> rc = smk_curacc(ssp, access, &ad);
> rc = smk_bu_current("shm", ssp, access, rc);
> @@ -3012,7 +3012,7 @@ static int smk_curacc_shm(struct shmid_kernel *shp, int access)
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smack_shm_associate(struct shmid_kernel *shp, int shmflg)
> +static int smack_shm_associate(struct kern_ipc_perm *shp, int shmflg)
> {
> int may;
>
> @@ -3027,7 +3027,7 @@ static int smack_shm_associate(struct shmid_kernel *shp, int shmflg)
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smack_shm_shmctl(struct shmid_kernel *shp, int cmd)
> +static int smack_shm_shmctl(struct kern_ipc_perm *shp, int cmd)
> {
> int may;
>
> @@ -3062,7 +3062,7 @@ static int smack_shm_shmctl(struct shmid_kernel *shp, int cmd)
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smack_shm_shmat(struct shmid_kernel *shp, char __user *shmaddr,
> +static int smack_shm_shmat(struct kern_ipc_perm *shp, char __user *shmaddr,
> int shmflg)
> {
> int may;
^ permalink raw reply
* Re: [REVIEW][PATCH 01/11] sem/security: Pass kern_ipc_perm not sem_array into the sem security hooks
From: Casey Schaufler @ 2018-03-23 21:46 UTC (permalink / raw)
To: Eric W. Biederman, Linux Containers
Cc: linux-kernel, linux-api, khlebnikov, prakash.sangappa, luto, akpm,
oleg, serge.hallyn, esyr, jannh, linux-security-module,
Pavel Emelyanov, Nagarathnam Muthusamy
In-Reply-To: <20180323191614.32489-1-ebiederm@xmission.com>
On 3/23/2018 12:16 PM, Eric W. Biederman wrote:
> All of the implementations of security hooks that take sem_array only
> access sem_perm the struct kern_ipc_perm member. This means the
> dependencies of the sem security hooks can be simplified by passing
> the kern_ipc_perm member of sem_array.
>
> Making this change will allow struct sem and struct sem_array
> to become private to ipc/sem.c.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
> include/linux/lsm_hooks.h | 10 +++++-----
> include/linux/security.h | 21 ++++++++++-----------
> ipc/sem.c | 19 ++++++++-----------
> security/security.c | 10 +++++-----
> security/selinux/hooks.c | 28 ++++++++++++++--------------
> security/smack/smack_lsm.c | 22 +++++++++++-----------
> 6 files changed, 53 insertions(+), 57 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 7161d8e7ee79..e4a94863a88c 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1592,11 +1592,11 @@ union security_list_options {
> int (*shm_shmat)(struct shmid_kernel *shp, char __user *shmaddr,
> int shmflg);
>
> - int (*sem_alloc_security)(struct sem_array *sma);
> - void (*sem_free_security)(struct sem_array *sma);
> - int (*sem_associate)(struct sem_array *sma, int semflg);
> - int (*sem_semctl)(struct sem_array *sma, int cmd);
> - int (*sem_semop)(struct sem_array *sma, struct sembuf *sops,
> + int (*sem_alloc_security)(struct kern_ipc_perm *sma);
> + void (*sem_free_security)(struct kern_ipc_perm *sma);
> + int (*sem_associate)(struct kern_ipc_perm *sma, int semflg);
> + int (*sem_semctl)(struct kern_ipc_perm *sma, int cmd);
> + int (*sem_semop)(struct kern_ipc_perm *sma, struct sembuf *sops,
> unsigned nsops, int alter);
>
> int (*netlink_send)(struct sock *sk, struct sk_buff *skb);
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 73f1ef625d40..fa7adac4b99a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -36,7 +36,6 @@ struct linux_binprm;
> struct cred;
> struct rlimit;
> struct siginfo;
> -struct sem_array;
> struct sembuf;
> struct kern_ipc_perm;
> struct audit_context;
> @@ -368,11 +367,11 @@ void security_shm_free(struct shmid_kernel *shp);
> int security_shm_associate(struct shmid_kernel *shp, int shmflg);
> int security_shm_shmctl(struct shmid_kernel *shp, int cmd);
> int security_shm_shmat(struct shmid_kernel *shp, char __user *shmaddr, int shmflg);
> -int security_sem_alloc(struct sem_array *sma);
> -void security_sem_free(struct sem_array *sma);
> -int security_sem_associate(struct sem_array *sma, int semflg);
> -int security_sem_semctl(struct sem_array *sma, int cmd);
> -int security_sem_semop(struct sem_array *sma, struct sembuf *sops,
> +int security_sem_alloc(struct kern_ipc_perm *sma);
> +void security_sem_free(struct kern_ipc_perm *sma);
> +int security_sem_associate(struct kern_ipc_perm *sma, int semflg);
> +int security_sem_semctl(struct kern_ipc_perm *sma, int cmd);
> +int security_sem_semop(struct kern_ipc_perm *sma, struct sembuf *sops,
> unsigned nsops, int alter);
> void security_d_instantiate(struct dentry *dentry, struct inode *inode);
> int security_getprocattr(struct task_struct *p, char *name, char **value);
> @@ -1103,25 +1102,25 @@ static inline int security_shm_shmat(struct shmid_kernel *shp,
> return 0;
> }
>
> -static inline int security_sem_alloc(struct sem_array *sma)
> +static inline int security_sem_alloc(struct kern_ipc_perm *sma)
> {
> return 0;
> }
>
> -static inline void security_sem_free(struct sem_array *sma)
> +static inline void security_sem_free(struct kern_ipc_perm *sma)
> { }
>
> -static inline int security_sem_associate(struct sem_array *sma, int semflg)
> +static inline int security_sem_associate(struct kern_ipc_perm *sma, int semflg)
> {
> return 0;
> }
>
> -static inline int security_sem_semctl(struct sem_array *sma, int cmd)
> +static inline int security_sem_semctl(struct kern_ipc_perm *sma, int cmd)
> {
> return 0;
> }
>
> -static inline int security_sem_semop(struct sem_array *sma,
> +static inline int security_sem_semop(struct kern_ipc_perm *sma,
> struct sembuf *sops, unsigned nsops,
> int alter)
> {
> diff --git a/ipc/sem.c b/ipc/sem.c
> index a4af04979fd2..01f5c63670ae 100644
> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -265,7 +265,7 @@ static void sem_rcu_free(struct rcu_head *head)
> struct kern_ipc_perm *p = container_of(head, struct kern_ipc_perm, rcu);
> struct sem_array *sma = container_of(p, struct sem_array, sem_perm);
>
> - security_sem_free(sma);
> + security_sem_free(&sma->sem_perm);
> kvfree(sma);
> }
>
> @@ -495,7 +495,7 @@ static int newary(struct ipc_namespace *ns, struct ipc_params *params)
> sma->sem_perm.key = key;
>
> sma->sem_perm.security = NULL;
> - retval = security_sem_alloc(sma);
> + retval = security_sem_alloc(&sma->sem_perm);
> if (retval) {
> kvfree(sma);
> return retval;
> @@ -535,10 +535,7 @@ static int newary(struct ipc_namespace *ns, struct ipc_params *params)
> */
> static inline int sem_security(struct kern_ipc_perm *ipcp, int semflg)
> {
> - struct sem_array *sma;
> -
> - sma = container_of(ipcp, struct sem_array, sem_perm);
> - return security_sem_associate(sma, semflg);
> + return security_sem_associate(ipcp, semflg);
> }
>
> /*
> @@ -1209,7 +1206,7 @@ static int semctl_stat(struct ipc_namespace *ns, int semid,
> if (ipcperms(ns, &sma->sem_perm, S_IRUGO))
> goto out_unlock;
>
> - err = security_sem_semctl(sma, cmd);
> + err = security_sem_semctl(&sma->sem_perm, cmd);
> if (err)
> goto out_unlock;
>
> @@ -1300,7 +1297,7 @@ static int semctl_setval(struct ipc_namespace *ns, int semid, int semnum,
> return -EACCES;
> }
>
> - err = security_sem_semctl(sma, SETVAL);
> + err = security_sem_semctl(&sma->sem_perm, SETVAL);
> if (err) {
> rcu_read_unlock();
> return -EACCES;
> @@ -1354,7 +1351,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
> if (ipcperms(ns, &sma->sem_perm, cmd == SETALL ? S_IWUGO : S_IRUGO))
> goto out_rcu_wakeup;
>
> - err = security_sem_semctl(sma, cmd);
> + err = security_sem_semctl(&sma->sem_perm, cmd);
> if (err)
> goto out_rcu_wakeup;
>
> @@ -1545,7 +1542,7 @@ static int semctl_down(struct ipc_namespace *ns, int semid,
>
> sma = container_of(ipcp, struct sem_array, sem_perm);
>
> - err = security_sem_semctl(sma, cmd);
> + err = security_sem_semctl(&sma->sem_perm, cmd);
> if (err)
> goto out_unlock1;
>
> @@ -1962,7 +1959,7 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops,
> goto out_free;
> }
>
> - error = security_sem_semop(sma, sops, nsops, alter);
> + error = security_sem_semop(&sma->sem_perm, sops, nsops, alter);
> if (error) {
> rcu_read_unlock();
> goto out_free;
> diff --git a/security/security.c b/security/security.c
> index 1cd8526cb0b7..d3b9aeb6b73b 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1220,27 +1220,27 @@ int security_shm_shmat(struct shmid_kernel *shp, char __user *shmaddr, int shmfl
> return call_int_hook(shm_shmat, 0, shp, shmaddr, shmflg);
> }
>
> -int security_sem_alloc(struct sem_array *sma)
> +int security_sem_alloc(struct kern_ipc_perm *sma)
> {
> return call_int_hook(sem_alloc_security, 0, sma);
> }
>
> -void security_sem_free(struct sem_array *sma)
> +void security_sem_free(struct kern_ipc_perm *sma)
> {
> call_void_hook(sem_free_security, sma);
> }
>
> -int security_sem_associate(struct sem_array *sma, int semflg)
> +int security_sem_associate(struct kern_ipc_perm *sma, int semflg)
> {
> return call_int_hook(sem_associate, 0, sma, semflg);
> }
>
> -int security_sem_semctl(struct sem_array *sma, int cmd)
> +int security_sem_semctl(struct kern_ipc_perm *sma, int cmd)
> {
> return call_int_hook(sem_semctl, 0, sma, cmd);
> }
>
> -int security_sem_semop(struct sem_array *sma, struct sembuf *sops,
> +int security_sem_semop(struct kern_ipc_perm *sma, struct sembuf *sops,
> unsigned nsops, int alter)
> {
> return call_int_hook(sem_semop, 0, sma, sops, nsops, alter);
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 8644d864e3c1..cce994e9fc0a 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -5767,53 +5767,53 @@ static int selinux_shm_shmat(struct shmid_kernel *shp,
> }
>
> /* Semaphore security operations */
> -static int selinux_sem_alloc_security(struct sem_array *sma)
> +static int selinux_sem_alloc_security(struct kern_ipc_perm *sma)
> {
> struct ipc_security_struct *isec;
> struct common_audit_data ad;
> u32 sid = current_sid();
> int rc;
>
> - rc = ipc_alloc_security(&sma->sem_perm, SECCLASS_SEM);
> + rc = ipc_alloc_security(sma, SECCLASS_SEM);
> if (rc)
> return rc;
>
> - isec = sma->sem_perm.security;
> + isec = sma->security;
>
> ad.type = LSM_AUDIT_DATA_IPC;
> - ad.u.ipc_id = sma->sem_perm.key;
> + ad.u.ipc_id = sma->key;
>
> rc = avc_has_perm(sid, isec->sid, SECCLASS_SEM,
> SEM__CREATE, &ad);
> if (rc) {
> - ipc_free_security(&sma->sem_perm);
> + ipc_free_security(sma);
> return rc;
> }
> return 0;
> }
>
> -static void selinux_sem_free_security(struct sem_array *sma)
> +static void selinux_sem_free_security(struct kern_ipc_perm *sma)
> {
> - ipc_free_security(&sma->sem_perm);
> + ipc_free_security(sma);
> }
>
> -static int selinux_sem_associate(struct sem_array *sma, int semflg)
> +static int selinux_sem_associate(struct kern_ipc_perm *sma, int semflg)
> {
> struct ipc_security_struct *isec;
> struct common_audit_data ad;
> u32 sid = current_sid();
>
> - isec = sma->sem_perm.security;
> + isec = sma->security;
>
> ad.type = LSM_AUDIT_DATA_IPC;
> - ad.u.ipc_id = sma->sem_perm.key;
> + ad.u.ipc_id = sma->key;
>
> return avc_has_perm(sid, isec->sid, SECCLASS_SEM,
> SEM__ASSOCIATE, &ad);
> }
>
> /* Note, at this point, sma is locked down */
> -static int selinux_sem_semctl(struct sem_array *sma, int cmd)
> +static int selinux_sem_semctl(struct kern_ipc_perm *sma, int cmd)
> {
> int err;
> u32 perms;
> @@ -5851,11 +5851,11 @@ static int selinux_sem_semctl(struct sem_array *sma, int cmd)
> return 0;
> }
>
> - err = ipc_has_perm(&sma->sem_perm, perms);
> + err = ipc_has_perm(sma, perms);
> return err;
> }
>
> -static int selinux_sem_semop(struct sem_array *sma,
> +static int selinux_sem_semop(struct kern_ipc_perm *sma,
> struct sembuf *sops, unsigned nsops, int alter)
> {
> u32 perms;
> @@ -5865,7 +5865,7 @@ static int selinux_sem_semop(struct sem_array *sma,
> else
> perms = SEM__READ;
>
> - return ipc_has_perm(&sma->sem_perm, perms);
> + return ipc_has_perm(sma, perms);
> }
>
> static int selinux_ipc_permission(struct kern_ipc_perm *ipcp, short flag)
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 03fdecba93bb..0402b8c1aec1 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -3077,9 +3077,9 @@ static int smack_shm_shmat(struct shmid_kernel *shp, char __user *shmaddr,
> *
> * Returns a pointer to the smack value
> */
> -static struct smack_known *smack_of_sem(struct sem_array *sma)
> +static struct smack_known *smack_of_sem(struct kern_ipc_perm *sma)
> {
> - return (struct smack_known *)sma->sem_perm.security;
> + return (struct smack_known *)sma->security;
> }
This function no longer makes sense and should be removed.
>
> /**
> @@ -3088,9 +3088,9 @@ static struct smack_known *smack_of_sem(struct sem_array *sma)
> *
> * Returns 0
> */
> -static int smack_sem_alloc_security(struct sem_array *sma)
> +static int smack_sem_alloc_security(struct kern_ipc_perm *sma)
> {
> - struct kern_ipc_perm *isp = &sma->sem_perm;
> + struct kern_ipc_perm *isp = sma;
> struct smack_known *skp = smk_of_current();
>
> isp->security = skp;
A kern_ipc_perm pointer is conventionally named isp in this code.
How about instead:
-static int smack_sem_alloc_security(struct sem_array *sma)
+static int smack_sem_alloc_security(struct kern_ipc_perm *isp)
{
- struct kern_ipc_perm *isp = &sma->sem_perm;
> @@ -3103,9 +3103,9 @@ static int smack_sem_alloc_security(struct sem_array *sma)
> *
> * Clears the blob pointer
> */
> -static void smack_sem_free_security(struct sem_array *sma)
> +static void smack_sem_free_security(struct kern_ipc_perm *sma)
> {
> - struct kern_ipc_perm *isp = &sma->sem_perm;
> + struct kern_ipc_perm *isp = sma;
-static void smack_sem_free_security(struct sem_array *sma)
+static void smack_sem_free_security(struct kern_ipc_perm *isp)
{
- struct kern_ipc_perm *isp = &sma->sem_perm;
>
> isp->security = NULL;
> }
> @@ -3117,7 +3117,7 @@ static void smack_sem_free_security(struct sem_array *sma)
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smk_curacc_sem(struct sem_array *sma, int access)
> +static int smk_curacc_sem(struct kern_ipc_perm *sma, int access)
-static int smk_curacc_sem(struct sem_array *sma, int access)
+static int smk_curacc_sem(struct kern_ipc_perm *isp, int access)
> {
> struct smack_known *ssp = smack_of_sem(sma);
struct smack_known *ssp = smack_of_ipc(isp);
> struct smk_audit_info ad;
> @@ -3125,7 +3125,7 @@ static int smk_curacc_sem(struct sem_array *sma, int access)
>
> #ifdef CONFIG_AUDIT
> smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_IPC);
> - ad.a.u.ipc_id = sma->sem_perm.id;
> + ad.a.u.ipc_id = sma->id;
- ad.a.u.ipc_id = isp->sem_perm.id;
+ ad.a.u.ipc_id = isp->id;
> #endif
> rc = smk_curacc(ssp, access, &ad);
> rc = smk_bu_current("sem", ssp, access, rc);
> @@ -3139,7 +3139,7 @@ static int smk_curacc_sem(struct sem_array *sma, int access)
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smack_sem_associate(struct sem_array *sma, int semflg)
> +static int smack_sem_associate(struct kern_ipc_perm *sma, int semflg)
+static int smack_sem_associate(struct kern_ipc_perm *isp, int semflg)
and related adjustments following.
> {
> int may;
>
> @@ -3154,7 +3154,7 @@ static int smack_sem_associate(struct sem_array *sma, int semflg)
> *
> * Returns 0 if current has the requested access, error code otherwise
> */
> -static int smack_sem_semctl(struct sem_array *sma, int cmd)
> +static int smack_sem_semctl(struct kern_ipc_perm *sma, int cmd)
+static int smack_sem_semctl(struct kern_ipc_perm *isp, int cmd)
and related adjustments following.
> {
> int may;
>
> @@ -3198,7 +3198,7 @@ static int smack_sem_semctl(struct sem_array *sma, int cmd)
> *
> * Returns 0 if access is allowed, error code otherwise
> */
> -static int smack_sem_semop(struct sem_array *sma, struct sembuf *sops,
> +static int smack_sem_semop(struct kern_ipc_perm *sma, struct sembuf *sops,
> unsigned nsops, int alter)
> {
> return smk_curacc_sem(sma, MAY_READWRITE);
+static int smack_sem_semop(struct kern_ipc_perm *isp, struct sembuf *sops,
unsigned nsops, int alter)
{
- return smk_curacc_sem(sma, MAY_READWRITE);
+ return smk_curacc_sem(isp, MAY_READWRITE);
^ permalink raw reply
* Re: [REVIEW][PATCH 09/11] ipc/shm: Fix shmctl(..., IPC_STAT, ...) between pid namespaces.
From: NAGARATHNAM MUTHUSAMY @ 2018-03-23 21:41 UTC (permalink / raw)
To: Eric W. Biederman
Cc: esyr-H+wXaHxf7aLQT0dZR+AlfA, jannh-hpIqsD4AKlfQT0dZR+AlfA,
khlebnikov-XoJtRXgx1JseBXzfvpsJ4g,
linux-api-u79uwXL29TY76Z2rM5mHXA, Linux Containers,
serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
prakash.sangappa-QHcLZuEGTsvQT0dZR+AlfA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
luto-DgEjT+Ai2ygdnm+yROfE0A, oleg-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Pavel Emelyanov
In-Reply-To: <87lgeio4tb.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
On 3/23/2018 2:33 PM, ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org wrote:
> NAGARATHNAM MUTHUSAMY <nagarathnam.muthusamy-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> writes:
>
>> Thanks!
>>
>> Reviewed-by: Nagarathnam Muthusamy <nagarathnam.muthusamy-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Does this look like it will address the issue you have been fighting
> with pids?
We do use IPC shared memory but it is a single large one, shared by multiple
levels. We are currently looking into using a similar solution based on
file locks.
When a new level is created, a file representing that level could be
created in
a common path which could be locked by the init process of that level.
Parent levels could query the locking pid of that file to get the pid
translation
of the init process of the required level. Then it could open a file
descriptor
and use the translate_pid API for further translations.
Thanks,
Nagarathnam.
>
> Eric
^ permalink raw reply
* Re: [REVIEW][PATCH 09/11] ipc/shm: Fix shmctl(..., IPC_STAT, ...) between pid namespaces.
From: Eric W. Biederman @ 2018-03-23 21:33 UTC (permalink / raw)
To: NAGARATHNAM MUTHUSAMY
Cc: esyr-H+wXaHxf7aLQT0dZR+AlfA, jannh-hpIqsD4AKlfQT0dZR+AlfA,
khlebnikov-XoJtRXgx1JseBXzfvpsJ4g,
linux-api-u79uwXL29TY76Z2rM5mHXA, Linux Containers,
serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
prakash.sangappa-QHcLZuEGTsvQT0dZR+AlfA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
luto-DgEjT+Ai2ygdnm+yROfE0A, oleg-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Pavel Emelyanov
In-Reply-To: <7df62190-2407-bfd4-d144-7304a8ea8ae3-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
NAGARATHNAM MUTHUSAMY <nagarathnam.muthusamy-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> writes:
> Thanks!
>
> Reviewed-by: Nagarathnam Muthusamy <nagarathnam.muthusamy-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Does this look like it will address the issue you have been fighting
with pids?
Eric
^ permalink raw reply
* Re: [REVIEW][PATCH 10/11] ipc/msg: Fix msgctl(..., IPC_STAT, ...) between pid namespaces
From: NAGARATHNAM MUTHUSAMY @ 2018-03-23 21:21 UTC (permalink / raw)
To: Eric W. Biederman, Linux Containers
Cc: esyr-H+wXaHxf7aLQT0dZR+AlfA, jannh-hpIqsD4AKlfQT0dZR+AlfA,
khlebnikov-XoJtRXgx1JseBXzfvpsJ4g,
linux-api-u79uwXL29TY76Z2rM5mHXA,
serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
prakash.sangappa-QHcLZuEGTsvQT0dZR+AlfA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
luto-DgEjT+Ai2ygdnm+yROfE0A, oleg-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Pavel Emelyanov
In-Reply-To: <20180323191614.32489-10-ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
On 3/23/2018 12:16 PM, Eric W. Biederman wrote:
> Today msg_lspid and msg_lrpid are remembered in the pid namespace of
> the creator and the processes that last send or received a sysvipc
> message. If you have processes in multiple pid namespaces that is
> just wrong. The process ids reported will not make the least bit of
> sense.
>
> This fix is slightly more susceptible to a performance problem than
> the related fix for System V shared memory. By definition the pids
> are updated by msgsnd and msgrcv, the fast path of System V message
> queues. The only concern over the previous implementation is the
> incrementing and decrementing of the pid reference count. As that is
> the only difference and multiple updates by of the task_tgid by
> threads in the same process have been shown in af_unix sockets to
> create a cache line ping-pong between cpus of the same processor.
>
> In this case I don't expect cache lines holding pid reference counts
> to ping pong between cpus. As senders and receivers update different
> pids there is a natural separation there. Further if multiple threads
> of the same process either send or receive messages the pid will be
> updated to the same value and ipc_update_pid will avoid the reference
> count update.
>
> Which means in the common case I expect msg_lspid and msg_lrpid to
> remain constant, and reference counts not to be updated when messages
> are sent.
>
> In rare cases it may be possible to trigger the issue which was
> observed for af_unix sockets, but it will require multiple processes
> with multiple threads to be either sending or receiving messages. It
> just does not feel likely that anyone would do that in practice.
>
> This change updates msgctl(..., IPC_STAT, ...) to return msg_lspid and
> msg_lrpid in the pid namespace of the process calling stat.
>
> This change also updates cat /proc/sysvipc/msg to return print msg_lspid
> and msg_lrpid in the pid namespace of the process that opened the proc
> file.
>
> Fixes: b488893a390e ("pid namespaces: changes to show virtual ids to user")
> Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
Thanks!
Reviewed-by: Nagarathnam Muthusamy <nagarathnam.muthusamy-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
> ipc/msg.c | 23 +++++++++++++----------
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/ipc/msg.c b/ipc/msg.c
> index af5a963306c4..825ad585a6ff 100644
> --- a/ipc/msg.c
> +++ b/ipc/msg.c
> @@ -52,8 +52,8 @@ struct msg_queue {
> unsigned long q_cbytes; /* current number of bytes on queue */
> unsigned long q_qnum; /* number of messages in queue */
> unsigned long q_qbytes; /* max number of bytes on queue */
> - pid_t q_lspid; /* pid of last msgsnd */
> - pid_t q_lrpid; /* last receive pid */
> + struct pid *q_lspid; /* pid of last msgsnd */
> + struct pid *q_lrpid; /* last receive pid */
>
> struct list_head q_messages;
> struct list_head q_receivers;
> @@ -154,7 +154,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
> msq->q_ctime = ktime_get_real_seconds();
> msq->q_cbytes = msq->q_qnum = 0;
> msq->q_qbytes = ns->msg_ctlmnb;
> - msq->q_lspid = msq->q_lrpid = 0;
> + msq->q_lspid = msq->q_lrpid = NULL;
> INIT_LIST_HEAD(&msq->q_messages);
> INIT_LIST_HEAD(&msq->q_receivers);
> INIT_LIST_HEAD(&msq->q_senders);
> @@ -267,6 +267,8 @@ static void freeque(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp)
> free_msg(msg);
> }
> atomic_sub(msq->q_cbytes, &ns->msg_bytes);
> + ipc_update_pid(&msq->q_lspid, NULL);
> + ipc_update_pid(&msq->q_lrpid, NULL);
> ipc_rcu_putref(&msq->q_perm, msg_rcu_free);
> }
>
> @@ -536,8 +538,8 @@ static int msgctl_stat(struct ipc_namespace *ns, int msqid,
> p->msg_cbytes = msq->q_cbytes;
> p->msg_qnum = msq->q_qnum;
> p->msg_qbytes = msq->q_qbytes;
> - p->msg_lspid = msq->q_lspid;
> - p->msg_lrpid = msq->q_lrpid;
> + p->msg_lspid = pid_vnr(msq->q_lspid);
> + p->msg_lrpid = pid_vnr(msq->q_lrpid);
>
> ipc_unlock_object(&msq->q_perm);
> rcu_read_unlock();
> @@ -741,7 +743,7 @@ static inline int pipelined_send(struct msg_queue *msq, struct msg_msg *msg,
> wake_q_add(wake_q, msr->r_tsk);
> WRITE_ONCE(msr->r_msg, ERR_PTR(-E2BIG));
> } else {
> - msq->q_lrpid = task_pid_vnr(msr->r_tsk);
> + ipc_update_pid(&msq->q_lrpid, task_pid(msr->r_tsk));
> msq->q_rtime = get_seconds();
>
> wake_q_add(wake_q, msr->r_tsk);
> @@ -842,7 +844,7 @@ static long do_msgsnd(int msqid, long mtype, void __user *mtext,
>
> }
>
> - msq->q_lspid = task_tgid_vnr(current);
> + ipc_update_pid(&msq->q_lspid, task_tgid(current));
> msq->q_stime = get_seconds();
>
> if (!pipelined_send(msq, msg, &wake_q)) {
> @@ -1060,7 +1062,7 @@ static long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, in
> list_del(&msg->m_list);
> msq->q_qnum--;
> msq->q_rtime = get_seconds();
> - msq->q_lrpid = task_tgid_vnr(current);
> + ipc_update_pid(&msq->q_lrpid, task_tgid(current));
> msq->q_cbytes -= msg->m_ts;
> atomic_sub(msg->m_ts, &ns->msg_bytes);
> atomic_dec(&ns->msg_hdrs);
> @@ -1202,6 +1204,7 @@ void msg_exit_ns(struct ipc_namespace *ns)
> #ifdef CONFIG_PROC_FS
> static int sysvipc_msg_proc_show(struct seq_file *s, void *it)
> {
> + struct pid_namespace *pid_ns = ipc_seq_pid_ns(s);
> struct user_namespace *user_ns = seq_user_ns(s);
> struct kern_ipc_perm *ipcp = it;
> struct msg_queue *msq = container_of(ipcp, struct msg_queue, q_perm);
> @@ -1213,8 +1216,8 @@ static int sysvipc_msg_proc_show(struct seq_file *s, void *it)
> msq->q_perm.mode,
> msq->q_cbytes,
> msq->q_qnum,
> - msq->q_lspid,
> - msq->q_lrpid,
> + pid_nr_ns(msq->q_lspid, pid_ns),
> + pid_nr_ns(msq->q_lrpid, pid_ns),
> from_kuid_munged(user_ns, msq->q_perm.uid),
> from_kgid_munged(user_ns, msq->q_perm.gid),
> from_kuid_munged(user_ns, msq->q_perm.cuid),
^ permalink raw reply
* Re: [REVIEW][PATCH 09/11] ipc/shm: Fix shmctl(..., IPC_STAT, ...) between pid namespaces.
From: NAGARATHNAM MUTHUSAMY @ 2018-03-23 21:17 UTC (permalink / raw)
To: Eric W. Biederman, Linux Containers
Cc: esyr-H+wXaHxf7aLQT0dZR+AlfA, jannh-hpIqsD4AKlfQT0dZR+AlfA,
khlebnikov-XoJtRXgx1JseBXzfvpsJ4g,
linux-api-u79uwXL29TY76Z2rM5mHXA,
serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
prakash.sangappa-QHcLZuEGTsvQT0dZR+AlfA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
luto-DgEjT+Ai2ygdnm+yROfE0A, oleg-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Pavel Emelyanov
In-Reply-To: <20180323191614.32489-9-ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
On 3/23/2018 12:16 PM, Eric W. Biederman wrote:
> Today shm_cpid and shm_lpid are remembered in the pid namespace of the
> creator and the processes that last touched a sysvipc shared memory
> segment. If you have processes in multiple pid namespaces that
> is just wrong, and I don't know how this has been over-looked for
> so long.
>
> As only creation and shared memory attach and shared memory detach
> update the pids I do not expect there to be a repeat of the issues
> when struct pid was attached to each af_unix skb, which in some
> notable cases cut the performance in half. The problem was threads of
> the same process updating same struct pid from different cpus causing
> the cache line to be highly contended and bounce between cpus.
>
> As creation, attach, and detach are expected to be rare operations for
> sysvipc shared memory segments I do not expect that kind of cache line
> ping pong to cause probems. In addition because the pid is at a fixed
> location in the structure instead of being dynamic on a skb, the
> reference count of the pid does not need to be updated on each
> operation if the pid is the same. This ability to simply skip the pid
> reference count changes if the pid is unchanging further reduces the
> likelihood of the a cache line holding a pid reference count
> ping-ponging between cpus.
>
> Fixes: b488893a390e ("pid namespaces: changes to show virtual ids to user")
> Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
Thanks!
Reviewed-by: Nagarathnam Muthusamy <nagarathnam.muthusamy-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
> ipc/shm.c | 25 +++++++++++++++----------
> 1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/ipc/shm.c b/ipc/shm.c
> index 0565669ebe5c..932b7e411c6c 100644
> --- a/ipc/shm.c
> +++ b/ipc/shm.c
> @@ -57,8 +57,8 @@ struct shmid_kernel /* private to the kernel */
> time64_t shm_atim;
> time64_t shm_dtim;
> time64_t shm_ctim;
> - pid_t shm_cprid;
> - pid_t shm_lprid;
> + struct pid *shm_cprid;
> + struct pid *shm_lprid;
> struct user_struct *mlock_user;
>
> /* The task created the shm object. NULL if the task is dead. */
> @@ -226,7 +226,7 @@ static int __shm_open(struct vm_area_struct *vma)
> return PTR_ERR(shp);
>
> shp->shm_atim = ktime_get_real_seconds();
> - shp->shm_lprid = task_tgid_vnr(current);
> + ipc_update_pid(&shp->shm_lprid, task_tgid(current));
> shp->shm_nattch++;
> shm_unlock(shp);
> return 0;
> @@ -267,6 +267,8 @@ static void shm_destroy(struct ipc_namespace *ns, struct shmid_kernel *shp)
> user_shm_unlock(i_size_read(file_inode(shm_file)),
> shp->mlock_user);
> fput(shm_file);
> + ipc_update_pid(&shp->shm_cprid, NULL);
> + ipc_update_pid(&shp->shm_lprid, NULL);
> ipc_rcu_putref(&shp->shm_perm, shm_rcu_free);
> }
>
> @@ -311,7 +313,7 @@ static void shm_close(struct vm_area_struct *vma)
> if (WARN_ON_ONCE(IS_ERR(shp)))
> goto done; /* no-op */
>
> - shp->shm_lprid = task_tgid_vnr(current);
> + ipc_update_pid(&shp->shm_lprid, task_tgid(current));
> shp->shm_dtim = ktime_get_real_seconds();
> shp->shm_nattch--;
> if (shm_may_destroy(ns, shp))
> @@ -614,8 +616,8 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
> if (IS_ERR(file))
> goto no_file;
>
> - shp->shm_cprid = task_tgid_vnr(current);
> - shp->shm_lprid = 0;
> + shp->shm_cprid = get_pid(task_tgid(current));
> + shp->shm_lprid = NULL;
> shp->shm_atim = shp->shm_dtim = 0;
> shp->shm_ctim = ktime_get_real_seconds();
> shp->shm_segsz = size;
> @@ -648,6 +650,8 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
> user_shm_unlock(size, shp->mlock_user);
> fput(file);
> no_file:
> + ipc_update_pid(&shp->shm_cprid, NULL);
> + ipc_update_pid(&shp->shm_lprid, NULL);
> call_rcu(&shp->shm_perm.rcu, shm_rcu_free);
> return error;
> }
> @@ -970,8 +974,8 @@ static int shmctl_stat(struct ipc_namespace *ns, int shmid,
> tbuf->shm_atime = shp->shm_atim;
> tbuf->shm_dtime = shp->shm_dtim;
> tbuf->shm_ctime = shp->shm_ctim;
> - tbuf->shm_cpid = shp->shm_cprid;
> - tbuf->shm_lpid = shp->shm_lprid;
> + tbuf->shm_cpid = pid_vnr(shp->shm_cprid);
> + tbuf->shm_lpid = pid_vnr(shp->shm_lprid);
> tbuf->shm_nattch = shp->shm_nattch;
>
> ipc_unlock_object(&shp->shm_perm);
> @@ -1605,6 +1609,7 @@ SYSCALL_DEFINE1(shmdt, char __user *, shmaddr)
> #ifdef CONFIG_PROC_FS
> static int sysvipc_shm_proc_show(struct seq_file *s, void *it)
> {
> + struct pid_namespace *pid_ns = ipc_seq_pid_ns(s);
> struct user_namespace *user_ns = seq_user_ns(s);
> struct kern_ipc_perm *ipcp = it;
> struct shmid_kernel *shp;
> @@ -1627,8 +1632,8 @@ static int sysvipc_shm_proc_show(struct seq_file *s, void *it)
> shp->shm_perm.id,
> shp->shm_perm.mode,
> shp->shm_segsz,
> - shp->shm_cprid,
> - shp->shm_lprid,
> + pid_nr_ns(shp->shm_cprid, pid_ns),
> + pid_nr_ns(shp->shm_lprid, pid_ns),
> shp->shm_nattch,
> from_kuid_munged(user_ns, shp->shm_perm.uid),
> from_kgid_munged(user_ns, shp->shm_perm.gid),
^ permalink raw reply
* [REVIEW][PATCH 11/11] ipc/sem: Fix semctl(..., GETPID, ...) between pid namespaces
From: Eric W. Biederman @ 2018-03-23 19:16 UTC (permalink / raw)
To: Linux Containers
Cc: esyr-H+wXaHxf7aLQT0dZR+AlfA, jannh-hpIqsD4AKlfQT0dZR+AlfA,
khlebnikov-XoJtRXgx1JseBXzfvpsJ4g,
linux-api-u79uwXL29TY76Z2rM5mHXA,
serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
prakash.sangappa-QHcLZuEGTsvQT0dZR+AlfA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA, Eric W. Biederman,
luto-DgEjT+Ai2ygdnm+yROfE0A, oleg-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Nagarathnam Muthusamy,
Pavel Emelyanov
In-Reply-To: <87vadmobdw.fsf_-_-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
Today the last process to update a semaphore is remembered and
reported in the pid namespace of that process. If there are processes
in any other pid namespace querying that process id with GETPID the
result will be unusable nonsense as it does not make any
sense in your own pid namespace.
Due to ipc_update_pid I don't think you will be able to get System V
ipc semaphores into a troublesome cache line ping-pong. Using struct
pids from separate process are not a problem because they do not share
a cache line. Using struct pid from different threads of the same
process are unlikely to be a problem as the reference count update
can be avoided.
Further linux futexes are a much better tool for the job of mutual
exclusion between processes than System V semaphores. So I expect
programs that are performance limited by their interprocess mutual
exclusion primitive will be using futexes.
So while it is possible that enhancing the storage of the last
rocess of a System V semaphore from an integer to a struct pid
will cause a performance regression because of the effect
of frequently updating the pid reference count. I don't expect
that to happen in practice.
This change updates semctl(..., GETPID, ...) to return the
process id of the last process to update a semphore inthe
pid namespace of the calling process.
Fixes: b488893a390e ("pid namespaces: changes to show virtual ids to user")
Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
ipc/sem.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/ipc/sem.c b/ipc/sem.c
index d661c491b0a5..47b263960524 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -98,7 +98,7 @@ struct sem {
* - semctl, via SETVAL and SETALL.
* - at task exit when performing undo adjustments (see exit_sem).
*/
- int sempid;
+ struct pid *sempid;
spinlock_t lock; /* spinlock for fine-grained semtimedop */
struct list_head pending_alter; /* pending single-sop operations */
/* that alter the semaphore */
@@ -128,7 +128,7 @@ struct sem_queue {
struct list_head list; /* queue of pending operations */
struct task_struct *sleeper; /* this process */
struct sem_undo *undo; /* undo structure */
- int pid; /* process id of requesting process */
+ struct pid *pid; /* process id of requesting process */
int status; /* completion status of operation */
struct sembuf *sops; /* array of pending operations */
struct sembuf *blocking; /* the operation that blocked */
@@ -628,7 +628,8 @@ SYSCALL_DEFINE3(semget, key_t, key, int, nsems, int, semflg)
*/
static int perform_atomic_semop_slow(struct sem_array *sma, struct sem_queue *q)
{
- int result, sem_op, nsops, pid;
+ int result, sem_op, nsops;
+ struct pid *pid;
struct sembuf *sop;
struct sem *curr;
struct sembuf *sops;
@@ -666,7 +667,7 @@ static int perform_atomic_semop_slow(struct sem_array *sma, struct sem_queue *q)
sop--;
pid = q->pid;
while (sop >= sops) {
- sma->sems[sop->sem_num].sempid = pid;
+ ipc_update_pid(&sma->sems[sop->sem_num].sempid, pid);
sop--;
}
@@ -753,7 +754,7 @@ static int perform_atomic_semop(struct sem_array *sma, struct sem_queue *q)
un->semadj[sop->sem_num] = undo;
}
curr->semval += sem_op;
- curr->sempid = q->pid;
+ ipc_update_pid(&curr->sempid, q->pid);
}
return 0;
@@ -1160,6 +1161,7 @@ static void freeary(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp)
unlink_queue(sma, q);
wake_up_sem_queue_prepare(q, -EIDRM, &wake_q);
}
+ ipc_update_pid(&sem->sempid, NULL);
}
/* Remove the semaphore set from the IDR */
@@ -1352,7 +1354,7 @@ static int semctl_setval(struct ipc_namespace *ns, int semid, int semnum,
un->semadj[semnum] = 0;
curr->semval = val;
- curr->sempid = task_tgid_vnr(current);
+ ipc_update_pid(&curr->sempid, task_tgid(current));
sma->sem_ctime = ktime_get_real_seconds();
/* maybe some queued-up processes were waiting for this */
do_smart_update(sma, NULL, 0, 0, &wake_q);
@@ -1473,7 +1475,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
for (i = 0; i < nsems; i++) {
sma->sems[i].semval = sem_io[i];
- sma->sems[i].sempid = task_tgid_vnr(current);
+ ipc_update_pid(&sma->sems[i].sempid, task_tgid(current));
}
ipc_assert_locked_object(&sma->sem_perm);
@@ -1505,7 +1507,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum,
err = curr->semval;
goto out_unlock;
case GETPID:
- err = curr->sempid;
+ err = pid_vnr(curr->sempid);
goto out_unlock;
case GETNCNT:
err = count_semcnt(sma, semnum, 0);
@@ -2024,7 +2026,7 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops,
queue.sops = sops;
queue.nsops = nsops;
queue.undo = un;
- queue.pid = task_tgid_vnr(current);
+ queue.pid = task_tgid(current);
queue.alter = alter;
queue.dupsop = dupsop;
@@ -2318,7 +2320,7 @@ void exit_sem(struct task_struct *tsk)
semaphore->semval = 0;
if (semaphore->semval > SEMVMX)
semaphore->semval = SEMVMX;
- semaphore->sempid = task_tgid_vnr(current);
+ ipc_update_pid(&semaphore->sempid, task_tgid(current));
}
}
/* maybe some queued-up processes were waiting for this */
--
2.14.1
^ permalink raw reply related
* [REVIEW][PATCH 10/11] ipc/msg: Fix msgctl(..., IPC_STAT, ...) between pid namespaces
From: Eric W. Biederman @ 2018-03-23 19:16 UTC (permalink / raw)
To: Linux Containers
Cc: linux-kernel, linux-api, khlebnikov, prakash.sangappa, luto, akpm,
oleg, serge.hallyn, esyr, jannh, linux-security-module,
Pavel Emelyanov, Nagarathnam Muthusamy, Eric W. Biederman
In-Reply-To: <87vadmobdw.fsf_-_@xmission.com>
Today msg_lspid and msg_lrpid are remembered in the pid namespace of
the creator and the processes that last send or received a sysvipc
message. If you have processes in multiple pid namespaces that is
just wrong. The process ids reported will not make the least bit of
sense.
This fix is slightly more susceptible to a performance problem than
the related fix for System V shared memory. By definition the pids
are updated by msgsnd and msgrcv, the fast path of System V message
queues. The only concern over the previous implementation is the
incrementing and decrementing of the pid reference count. As that is
the only difference and multiple updates by of the task_tgid by
threads in the same process have been shown in af_unix sockets to
create a cache line ping-pong between cpus of the same processor.
In this case I don't expect cache lines holding pid reference counts
to ping pong between cpus. As senders and receivers update different
pids there is a natural separation there. Further if multiple threads
of the same process either send or receive messages the pid will be
updated to the same value and ipc_update_pid will avoid the reference
count update.
Which means in the common case I expect msg_lspid and msg_lrpid to
remain constant, and reference counts not to be updated when messages
are sent.
In rare cases it may be possible to trigger the issue which was
observed for af_unix sockets, but it will require multiple processes
with multiple threads to be either sending or receiving messages. It
just does not feel likely that anyone would do that in practice.
This change updates msgctl(..., IPC_STAT, ...) to return msg_lspid and
msg_lrpid in the pid namespace of the process calling stat.
This change also updates cat /proc/sysvipc/msg to return print msg_lspid
and msg_lrpid in the pid namespace of the process that opened the proc
file.
Fixes: b488893a390e ("pid namespaces: changes to show virtual ids to user")
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
ipc/msg.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/ipc/msg.c b/ipc/msg.c
index af5a963306c4..825ad585a6ff 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -52,8 +52,8 @@ struct msg_queue {
unsigned long q_cbytes; /* current number of bytes on queue */
unsigned long q_qnum; /* number of messages in queue */
unsigned long q_qbytes; /* max number of bytes on queue */
- pid_t q_lspid; /* pid of last msgsnd */
- pid_t q_lrpid; /* last receive pid */
+ struct pid *q_lspid; /* pid of last msgsnd */
+ struct pid *q_lrpid; /* last receive pid */
struct list_head q_messages;
struct list_head q_receivers;
@@ -154,7 +154,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
msq->q_ctime = ktime_get_real_seconds();
msq->q_cbytes = msq->q_qnum = 0;
msq->q_qbytes = ns->msg_ctlmnb;
- msq->q_lspid = msq->q_lrpid = 0;
+ msq->q_lspid = msq->q_lrpid = NULL;
INIT_LIST_HEAD(&msq->q_messages);
INIT_LIST_HEAD(&msq->q_receivers);
INIT_LIST_HEAD(&msq->q_senders);
@@ -267,6 +267,8 @@ static void freeque(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp)
free_msg(msg);
}
atomic_sub(msq->q_cbytes, &ns->msg_bytes);
+ ipc_update_pid(&msq->q_lspid, NULL);
+ ipc_update_pid(&msq->q_lrpid, NULL);
ipc_rcu_putref(&msq->q_perm, msg_rcu_free);
}
@@ -536,8 +538,8 @@ static int msgctl_stat(struct ipc_namespace *ns, int msqid,
p->msg_cbytes = msq->q_cbytes;
p->msg_qnum = msq->q_qnum;
p->msg_qbytes = msq->q_qbytes;
- p->msg_lspid = msq->q_lspid;
- p->msg_lrpid = msq->q_lrpid;
+ p->msg_lspid = pid_vnr(msq->q_lspid);
+ p->msg_lrpid = pid_vnr(msq->q_lrpid);
ipc_unlock_object(&msq->q_perm);
rcu_read_unlock();
@@ -741,7 +743,7 @@ static inline int pipelined_send(struct msg_queue *msq, struct msg_msg *msg,
wake_q_add(wake_q, msr->r_tsk);
WRITE_ONCE(msr->r_msg, ERR_PTR(-E2BIG));
} else {
- msq->q_lrpid = task_pid_vnr(msr->r_tsk);
+ ipc_update_pid(&msq->q_lrpid, task_pid(msr->r_tsk));
msq->q_rtime = get_seconds();
wake_q_add(wake_q, msr->r_tsk);
@@ -842,7 +844,7 @@ static long do_msgsnd(int msqid, long mtype, void __user *mtext,
}
- msq->q_lspid = task_tgid_vnr(current);
+ ipc_update_pid(&msq->q_lspid, task_tgid(current));
msq->q_stime = get_seconds();
if (!pipelined_send(msq, msg, &wake_q)) {
@@ -1060,7 +1062,7 @@ static long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, in
list_del(&msg->m_list);
msq->q_qnum--;
msq->q_rtime = get_seconds();
- msq->q_lrpid = task_tgid_vnr(current);
+ ipc_update_pid(&msq->q_lrpid, task_tgid(current));
msq->q_cbytes -= msg->m_ts;
atomic_sub(msg->m_ts, &ns->msg_bytes);
atomic_dec(&ns->msg_hdrs);
@@ -1202,6 +1204,7 @@ void msg_exit_ns(struct ipc_namespace *ns)
#ifdef CONFIG_PROC_FS
static int sysvipc_msg_proc_show(struct seq_file *s, void *it)
{
+ struct pid_namespace *pid_ns = ipc_seq_pid_ns(s);
struct user_namespace *user_ns = seq_user_ns(s);
struct kern_ipc_perm *ipcp = it;
struct msg_queue *msq = container_of(ipcp, struct msg_queue, q_perm);
@@ -1213,8 +1216,8 @@ static int sysvipc_msg_proc_show(struct seq_file *s, void *it)
msq->q_perm.mode,
msq->q_cbytes,
msq->q_qnum,
- msq->q_lspid,
- msq->q_lrpid,
+ pid_nr_ns(msq->q_lspid, pid_ns),
+ pid_nr_ns(msq->q_lrpid, pid_ns),
from_kuid_munged(user_ns, msq->q_perm.uid),
from_kgid_munged(user_ns, msq->q_perm.gid),
from_kuid_munged(user_ns, msq->q_perm.cuid),
--
2.14.1
^ permalink raw reply related
* [REVIEW][PATCH 09/11] ipc/shm: Fix shmctl(..., IPC_STAT, ...) between pid namespaces.
From: Eric W. Biederman @ 2018-03-23 19:16 UTC (permalink / raw)
To: Linux Containers
Cc: linux-kernel, linux-api, khlebnikov, prakash.sangappa, luto, akpm,
oleg, serge.hallyn, esyr, jannh, linux-security-module,
Pavel Emelyanov, Nagarathnam Muthusamy, Eric W. Biederman
In-Reply-To: <87vadmobdw.fsf_-_@xmission.com>
Today shm_cpid and shm_lpid are remembered in the pid namespace of the
creator and the processes that last touched a sysvipc shared memory
segment. If you have processes in multiple pid namespaces that
is just wrong, and I don't know how this has been over-looked for
so long.
As only creation and shared memory attach and shared memory detach
update the pids I do not expect there to be a repeat of the issues
when struct pid was attached to each af_unix skb, which in some
notable cases cut the performance in half. The problem was threads of
the same process updating same struct pid from different cpus causing
the cache line to be highly contended and bounce between cpus.
As creation, attach, and detach are expected to be rare operations for
sysvipc shared memory segments I do not expect that kind of cache line
ping pong to cause probems. In addition because the pid is at a fixed
location in the structure instead of being dynamic on a skb, the
reference count of the pid does not need to be updated on each
operation if the pid is the same. This ability to simply skip the pid
reference count changes if the pid is unchanging further reduces the
likelihood of the a cache line holding a pid reference count
ping-ponging between cpus.
Fixes: b488893a390e ("pid namespaces: changes to show virtual ids to user")
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
ipc/shm.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/ipc/shm.c b/ipc/shm.c
index 0565669ebe5c..932b7e411c6c 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -57,8 +57,8 @@ struct shmid_kernel /* private to the kernel */
time64_t shm_atim;
time64_t shm_dtim;
time64_t shm_ctim;
- pid_t shm_cprid;
- pid_t shm_lprid;
+ struct pid *shm_cprid;
+ struct pid *shm_lprid;
struct user_struct *mlock_user;
/* The task created the shm object. NULL if the task is dead. */
@@ -226,7 +226,7 @@ static int __shm_open(struct vm_area_struct *vma)
return PTR_ERR(shp);
shp->shm_atim = ktime_get_real_seconds();
- shp->shm_lprid = task_tgid_vnr(current);
+ ipc_update_pid(&shp->shm_lprid, task_tgid(current));
shp->shm_nattch++;
shm_unlock(shp);
return 0;
@@ -267,6 +267,8 @@ static void shm_destroy(struct ipc_namespace *ns, struct shmid_kernel *shp)
user_shm_unlock(i_size_read(file_inode(shm_file)),
shp->mlock_user);
fput(shm_file);
+ ipc_update_pid(&shp->shm_cprid, NULL);
+ ipc_update_pid(&shp->shm_lprid, NULL);
ipc_rcu_putref(&shp->shm_perm, shm_rcu_free);
}
@@ -311,7 +313,7 @@ static void shm_close(struct vm_area_struct *vma)
if (WARN_ON_ONCE(IS_ERR(shp)))
goto done; /* no-op */
- shp->shm_lprid = task_tgid_vnr(current);
+ ipc_update_pid(&shp->shm_lprid, task_tgid(current));
shp->shm_dtim = ktime_get_real_seconds();
shp->shm_nattch--;
if (shm_may_destroy(ns, shp))
@@ -614,8 +616,8 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
if (IS_ERR(file))
goto no_file;
- shp->shm_cprid = task_tgid_vnr(current);
- shp->shm_lprid = 0;
+ shp->shm_cprid = get_pid(task_tgid(current));
+ shp->shm_lprid = NULL;
shp->shm_atim = shp->shm_dtim = 0;
shp->shm_ctim = ktime_get_real_seconds();
shp->shm_segsz = size;
@@ -648,6 +650,8 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
user_shm_unlock(size, shp->mlock_user);
fput(file);
no_file:
+ ipc_update_pid(&shp->shm_cprid, NULL);
+ ipc_update_pid(&shp->shm_lprid, NULL);
call_rcu(&shp->shm_perm.rcu, shm_rcu_free);
return error;
}
@@ -970,8 +974,8 @@ static int shmctl_stat(struct ipc_namespace *ns, int shmid,
tbuf->shm_atime = shp->shm_atim;
tbuf->shm_dtime = shp->shm_dtim;
tbuf->shm_ctime = shp->shm_ctim;
- tbuf->shm_cpid = shp->shm_cprid;
- tbuf->shm_lpid = shp->shm_lprid;
+ tbuf->shm_cpid = pid_vnr(shp->shm_cprid);
+ tbuf->shm_lpid = pid_vnr(shp->shm_lprid);
tbuf->shm_nattch = shp->shm_nattch;
ipc_unlock_object(&shp->shm_perm);
@@ -1605,6 +1609,7 @@ SYSCALL_DEFINE1(shmdt, char __user *, shmaddr)
#ifdef CONFIG_PROC_FS
static int sysvipc_shm_proc_show(struct seq_file *s, void *it)
{
+ struct pid_namespace *pid_ns = ipc_seq_pid_ns(s);
struct user_namespace *user_ns = seq_user_ns(s);
struct kern_ipc_perm *ipcp = it;
struct shmid_kernel *shp;
@@ -1627,8 +1632,8 @@ static int sysvipc_shm_proc_show(struct seq_file *s, void *it)
shp->shm_perm.id,
shp->shm_perm.mode,
shp->shm_segsz,
- shp->shm_cprid,
- shp->shm_lprid,
+ pid_nr_ns(shp->shm_cprid, pid_ns),
+ pid_nr_ns(shp->shm_lprid, pid_ns),
shp->shm_nattch,
from_kuid_munged(user_ns, shp->shm_perm.uid),
from_kgid_munged(user_ns, shp->shm_perm.gid),
--
2.14.1
^ permalink raw reply related
* [REVIEW][PATCH 08/11] ipc/util: Helpers for making the sysvipc operations pid namespace aware
From: Eric W. Biederman @ 2018-03-23 19:16 UTC (permalink / raw)
To: Linux Containers
Cc: esyr-H+wXaHxf7aLQT0dZR+AlfA, jannh-hpIqsD4AKlfQT0dZR+AlfA,
khlebnikov-XoJtRXgx1JseBXzfvpsJ4g,
linux-api-u79uwXL29TY76Z2rM5mHXA,
serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
prakash.sangappa-QHcLZuEGTsvQT0dZR+AlfA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA, Eric W. Biederman,
luto-DgEjT+Ai2ygdnm+yROfE0A, oleg-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Nagarathnam Muthusamy,
Pavel Emelyanov
In-Reply-To: <87vadmobdw.fsf_-_-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
Capture the pid namespace when /proc/sysvipc/msg /proc/sysvipc/shm
and /proc/sysvipc/sem are opened, and make it available through
the new helper ipc_seq_pid_ns.
This makes it possible to report the pids in these files in the
pid namespace of the opener of the files.
Implement ipc_update_pid. A simple impline helper that will only update
a struct pid pointer if the new value does not equal the old value. This
removes the need for wordy code sequences like:
old = object->pid;
object->pid = new;
put_pid(old);
and
old = object->pid;
if (old != new) {
object->pid = new;
put_pid(old);
}
Allowing the following to be written instead:
ipc_update_pid(&object->pid, new);
Which is easier to read and ensures that the pid reference count is
not touched the old and the new values are the same. Not touching
the reference count in this case is important to help avoid issues
like af_unix experienced, where multiple threads of the same
process managed to bounce the struct pid between cpu cache lines,
but updating the pids reference count.
Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
ipc/util.c | 9 +++++++++
ipc/util.h | 11 +++++++++++
2 files changed, 20 insertions(+)
diff --git a/ipc/util.c b/ipc/util.c
index 4ed5a17dd06f..3783b7991cc7 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -747,9 +747,16 @@ int ipc_parse_version(int *cmd)
#ifdef CONFIG_PROC_FS
struct ipc_proc_iter {
struct ipc_namespace *ns;
+ struct pid_namespace *pid_ns;
struct ipc_proc_iface *iface;
};
+struct pid_namespace *ipc_seq_pid_ns(struct seq_file *s)
+{
+ struct ipc_proc_iter *iter = s->private;
+ return iter->pid_ns;
+}
+
/*
* This routine locks the ipc structure found at least at position pos.
*/
@@ -872,6 +879,7 @@ static int sysvipc_proc_open(struct inode *inode, struct file *file)
iter->iface = PDE_DATA(inode);
iter->ns = get_ipc_ns(current->nsproxy->ipc_ns);
+ iter->pid_ns = get_pid_ns(task_active_pid_ns(current));
return 0;
}
@@ -881,6 +889,7 @@ static int sysvipc_proc_release(struct inode *inode, struct file *file)
struct seq_file *seq = file->private_data;
struct ipc_proc_iter *iter = seq->private;
put_ipc_ns(iter->ns);
+ put_pid_ns(iter->pid_ns);
return seq_release_private(inode, file);
}
diff --git a/ipc/util.h b/ipc/util.h
index 959c10eb9cc1..e39ed9705f99 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -23,6 +23,7 @@ int msg_init(void);
void shm_init(void);
struct ipc_namespace;
+struct pid_namespace;
#ifdef CONFIG_POSIX_MQUEUE
extern void mq_clear_sbinfo(struct ipc_namespace *ns);
@@ -86,6 +87,7 @@ int ipc_init_ids(struct ipc_ids *);
#ifdef CONFIG_PROC_FS
void __init ipc_init_proc_interface(const char *path, const char *header,
int ids, int (*show)(struct seq_file *, void *));
+struct pid_namespace *ipc_seq_pid_ns(struct seq_file *);
#else
#define ipc_init_proc_interface(path, header, ids, show) do {} while (0)
#endif
@@ -150,6 +152,15 @@ struct kern_ipc_perm *ipcctl_pre_down_nolock(struct ipc_namespace *ns,
struct ipc_ids *ids, int id, int cmd,
struct ipc64_perm *perm, int extra_perm);
+static inline void ipc_update_pid(struct pid **pos, struct pid *pid)
+{
+ struct pid *old = *pos;
+ if (old != pid) {
+ *pos = get_pid(pid);
+ put_pid(old);
+ }
+}
+
#ifndef CONFIG_ARCH_WANT_IPC_PARSE_VERSION
/* On IA-64, we always use the "64-bit version" of the IPC structures. */
# define ipc_parse_version(cmd) IPC_64
--
2.14.1
^ permalink raw reply related
* [REVIEW][PATCH 07/11] ipc: Move IPCMNI from include/ipc.h into ipc/util.h
From: Eric W. Biederman @ 2018-03-23 19:16 UTC (permalink / raw)
To: Linux Containers
Cc: linux-kernel, linux-api, khlebnikov, prakash.sangappa, luto, akpm,
oleg, serge.hallyn, esyr, jannh, linux-security-module,
Pavel Emelyanov, Nagarathnam Muthusamy, Eric W. Biederman
In-Reply-To: <87vadmobdw.fsf_-_@xmission.com>
The definition IPCMNI is only used in ipc/util.h and ipc/util.c. So
there is no reason to keep it in a header file that the whole kernel
can see. Move it into util.h to simplify future maintenance.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
include/linux/ipc.h | 2 --
ipc/util.h | 1 +
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/linux/ipc.h b/include/linux/ipc.h
index 821b2f260992..6cc2df7f7ac9 100644
--- a/include/linux/ipc.h
+++ b/include/linux/ipc.h
@@ -8,8 +8,6 @@
#include <uapi/linux/ipc.h>
#include <linux/refcount.h>
-#define IPCMNI 32768 /* <= MAX_INT limit for ipc arrays (including sysctl changes) */
-
/* used by in-kernel data structures */
struct kern_ipc_perm {
spinlock_t lock;
diff --git a/ipc/util.h b/ipc/util.h
index 89b8ec176fc4..959c10eb9cc1 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -15,6 +15,7 @@
#include <linux/err.h>
#include <linux/ipc_namespace.h>
+#define IPCMNI 32768 /* <= MAX_INT limit for ipc arrays (including sysctl changes) */
#define SEQ_MULTIPLIER (IPCMNI)
int sem_init(void);
--
2.14.1
^ permalink raw reply related
* [REVIEW][PATCH 06/11] msg: Move struct msg_queue into ipc/msg.c
From: Eric W. Biederman @ 2018-03-23 19:16 UTC (permalink / raw)
To: Linux Containers
Cc: esyr-H+wXaHxf7aLQT0dZR+AlfA, jannh-hpIqsD4AKlfQT0dZR+AlfA,
khlebnikov-XoJtRXgx1JseBXzfvpsJ4g,
linux-api-u79uwXL29TY76Z2rM5mHXA,
serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
prakash.sangappa-QHcLZuEGTsvQT0dZR+AlfA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA, Eric W. Biederman,
luto-DgEjT+Ai2ygdnm+yROfE0A, oleg-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Nagarathnam Muthusamy,
Pavel Emelyanov
In-Reply-To: <87vadmobdw.fsf_-_-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
All of the users are now in ipc/msg.c so make the definition local to
that file to make code maintenance easier. AKA to prevent rebuilding
the entire kernel when struct msg_queue changes.
Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
include/linux/msg.h | 18 ------------------
ipc/msg.c | 17 +++++++++++++++++
2 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/include/linux/msg.h b/include/linux/msg.h
index 0a7eefeee0d1..9a972a296b95 100644
--- a/include/linux/msg.h
+++ b/include/linux/msg.h
@@ -3,7 +3,6 @@
#define _LINUX_MSG_H
#include <linux/list.h>
-#include <linux/time64.h>
#include <uapi/linux/msg.h>
/* one msg_msg structure for each message */
@@ -16,21 +15,4 @@ struct msg_msg {
/* the actual message follows immediately */
};
-/* one msq_queue structure for each present queue on the system */
-struct msg_queue {
- struct kern_ipc_perm q_perm;
- time64_t q_stime; /* last msgsnd time */
- time64_t q_rtime; /* last msgrcv time */
- time64_t q_ctime; /* last change time */
- unsigned long q_cbytes; /* current number of bytes on queue */
- unsigned long q_qnum; /* number of messages in queue */
- unsigned long q_qbytes; /* max number of bytes on queue */
- pid_t q_lspid; /* pid of last msgsnd */
- pid_t q_lrpid; /* last receive pid */
-
- struct list_head q_messages;
- struct list_head q_receivers;
- struct list_head q_senders;
-} __randomize_layout;
-
#endif /* _LINUX_MSG_H */
diff --git a/ipc/msg.c b/ipc/msg.c
index cdfab0825fce..af5a963306c4 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -43,6 +43,23 @@
#include <linux/uaccess.h>
#include "util.h"
+/* one msq_queue structure for each present queue on the system */
+struct msg_queue {
+ struct kern_ipc_perm q_perm;
+ time64_t q_stime; /* last msgsnd time */
+ time64_t q_rtime; /* last msgrcv time */
+ time64_t q_ctime; /* last change time */
+ unsigned long q_cbytes; /* current number of bytes on queue */
+ unsigned long q_qnum; /* number of messages in queue */
+ unsigned long q_qbytes; /* max number of bytes on queue */
+ pid_t q_lspid; /* pid of last msgsnd */
+ pid_t q_lrpid; /* last receive pid */
+
+ struct list_head q_messages;
+ struct list_head q_receivers;
+ struct list_head q_senders;
+} __randomize_layout;
+
/* one msg_receiver structure for each sleeping receiver */
struct msg_receiver {
struct list_head r_list;
--
2.14.1
^ permalink raw reply related
* [REVIEW][PATCH 05/11] shm: Move struct shmid_kernel into ipc/shm.c
From: Eric W. Biederman @ 2018-03-23 19:16 UTC (permalink / raw)
To: Linux Containers
Cc: linux-kernel, linux-api, khlebnikov, prakash.sangappa, luto, akpm,
oleg, serge.hallyn, esyr, jannh, linux-security-module,
Pavel Emelyanov, Nagarathnam Muthusamy, Eric W. Biederman
In-Reply-To: <87vadmobdw.fsf_-_@xmission.com>
All of the users are now in ipc/shm.c so make the definition local to
that file to make code maintenance easier. AKA to prevent rebuilding
the entire kernel when struct shmid_kernel changes.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
include/linux/shm.h | 23 -----------------------
ipc/shm.c | 22 ++++++++++++++++++++++
2 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/include/linux/shm.h b/include/linux/shm.h
index 2bbafacfbfc9..c7cb18405ad7 100644
--- a/include/linux/shm.h
+++ b/include/linux/shm.h
@@ -3,32 +3,9 @@
#define _LINUX_SHM_H_
#include <linux/list.h>
-#include <asm/page.h>
#include <uapi/linux/shm.h>
#include <asm/shmparam.h>
-struct shmid_kernel /* private to the kernel */
-{
- struct kern_ipc_perm shm_perm;
- struct file *shm_file;
- unsigned long shm_nattch;
- unsigned long shm_segsz;
- time64_t shm_atim;
- time64_t shm_dtim;
- time64_t shm_ctim;
- pid_t shm_cprid;
- pid_t shm_lprid;
- struct user_struct *mlock_user;
-
- /* The task created the shm object. NULL if the task is dead. */
- struct task_struct *shm_creator;
- struct list_head shm_clist; /* list by creator */
-} __randomize_layout;
-
-/* shm_mode upper byte flags */
-#define SHM_DEST 01000 /* segment will be destroyed on last detach */
-#define SHM_LOCKED 02000 /* segment will not be swapped */
-
#ifdef CONFIG_SYSVIPC
struct sysv_shm {
struct list_head shm_clist;
diff --git a/ipc/shm.c b/ipc/shm.c
index 387a786e7be1..0565669ebe5c 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -48,6 +48,28 @@
#include "util.h"
+struct shmid_kernel /* private to the kernel */
+{
+ struct kern_ipc_perm shm_perm;
+ struct file *shm_file;
+ unsigned long shm_nattch;
+ unsigned long shm_segsz;
+ time64_t shm_atim;
+ time64_t shm_dtim;
+ time64_t shm_ctim;
+ pid_t shm_cprid;
+ pid_t shm_lprid;
+ struct user_struct *mlock_user;
+
+ /* The task created the shm object. NULL if the task is dead. */
+ struct task_struct *shm_creator;
+ struct list_head shm_clist; /* list by creator */
+} __randomize_layout;
+
+/* shm_mode upper byte flags */
+#define SHM_DEST 01000 /* segment will be destroyed on last detach */
+#define SHM_LOCKED 02000 /* segment will not be swapped */
+
struct shm_file_data {
int id;
struct ipc_namespace *ns;
--
2.14.1
^ permalink raw reply related
* [REVIEW][PATCH 04/11] sem: Move struct sem and struct sem_array into ipc/sem.c
From: Eric W. Biederman @ 2018-03-23 19:16 UTC (permalink / raw)
To: Linux Containers
Cc: esyr-H+wXaHxf7aLQT0dZR+AlfA, jannh-hpIqsD4AKlfQT0dZR+AlfA,
khlebnikov-XoJtRXgx1JseBXzfvpsJ4g,
linux-api-u79uwXL29TY76Z2rM5mHXA,
serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
prakash.sangappa-QHcLZuEGTsvQT0dZR+AlfA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA, Eric W. Biederman,
luto-DgEjT+Ai2ygdnm+yROfE0A, oleg-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Nagarathnam Muthusamy,
Pavel Emelyanov
In-Reply-To: <87vadmobdw.fsf_-_-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
All of the users are now in ipc/sem.c so make the definitions
local to that file to make code maintenance easier. AKA
to prevent rebuilding the entire kernel when one of these
files is changed.
Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
include/linux/sem.h | 40 +---------------------------------------
ipc/sem.c | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+), 39 deletions(-)
diff --git a/include/linux/sem.h b/include/linux/sem.h
index 9badd322dcee..5608a500c43e 100644
--- a/include/linux/sem.h
+++ b/include/linux/sem.h
@@ -2,48 +2,10 @@
#ifndef _LINUX_SEM_H
#define _LINUX_SEM_H
-#include <linux/atomic.h>
-#include <linux/rcupdate.h>
-#include <linux/cache.h>
-#include <linux/time64.h>
#include <uapi/linux/sem.h>
struct task_struct;
-
-/* One semaphore structure for each semaphore in the system. */
-struct sem {
- int semval; /* current value */
- /*
- * PID of the process that last modified the semaphore. For
- * Linux, specifically these are:
- * - semop
- * - semctl, via SETVAL and SETALL.
- * - at task exit when performing undo adjustments (see exit_sem).
- */
- int sempid;
- spinlock_t lock; /* spinlock for fine-grained semtimedop */
- struct list_head pending_alter; /* pending single-sop operations */
- /* that alter the semaphore */
- struct list_head pending_const; /* pending single-sop operations */
- /* that do not alter the semaphore*/
- time_t sem_otime; /* candidate for sem_otime */
-} ____cacheline_aligned_in_smp;
-
-/* One sem_array data structure for each set of semaphores in the system. */
-struct sem_array {
- struct kern_ipc_perm sem_perm; /* permissions .. see ipc.h */
- time64_t sem_ctime; /* create/last semctl() time */
- struct list_head pending_alter; /* pending operations */
- /* that alter the array */
- struct list_head pending_const; /* pending complex operations */
- /* that do not alter semvals */
- struct list_head list_id; /* undo requests on this array */
- int sem_nsems; /* no. of semaphores in array */
- int complex_count; /* pending complex operations */
- unsigned int use_global_lock;/* >0: global lock required */
-
- struct sem sems[];
-} __randomize_layout;
+struct sem_undo_list;
#ifdef CONFIG_SYSVIPC
diff --git a/ipc/sem.c b/ipc/sem.c
index 01f5c63670ae..d661c491b0a5 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -88,6 +88,40 @@
#include <linux/uaccess.h>
#include "util.h"
+/* One semaphore structure for each semaphore in the system. */
+struct sem {
+ int semval; /* current value */
+ /*
+ * PID of the process that last modified the semaphore. For
+ * Linux, specifically these are:
+ * - semop
+ * - semctl, via SETVAL and SETALL.
+ * - at task exit when performing undo adjustments (see exit_sem).
+ */
+ int sempid;
+ spinlock_t lock; /* spinlock for fine-grained semtimedop */
+ struct list_head pending_alter; /* pending single-sop operations */
+ /* that alter the semaphore */
+ struct list_head pending_const; /* pending single-sop operations */
+ /* that do not alter the semaphore*/
+ time_t sem_otime; /* candidate for sem_otime */
+} ____cacheline_aligned_in_smp;
+
+/* One sem_array data structure for each set of semaphores in the system. */
+struct sem_array {
+ struct kern_ipc_perm sem_perm; /* permissions .. see ipc.h */
+ time64_t sem_ctime; /* create/last semctl() time */
+ struct list_head pending_alter; /* pending operations */
+ /* that alter the array */
+ struct list_head pending_const; /* pending complex operations */
+ /* that do not alter semvals */
+ struct list_head list_id; /* undo requests on this array */
+ int sem_nsems; /* no. of semaphores in array */
+ int complex_count; /* pending complex operations */
+ unsigned int use_global_lock;/* >0: global lock required */
+
+ struct sem sems[];
+} __randomize_layout;
/* One queue for each sleeping process in the system. */
struct sem_queue {
--
2.14.1
^ permalink raw reply related
* [REVIEW][PATCH 03/11] msg/security: Pass kern_ipc_perm not msg_queue into the msg_queue security hooks
From: Eric W. Biederman @ 2018-03-23 19:16 UTC (permalink / raw)
To: Linux Containers
Cc: linux-kernel, linux-api, khlebnikov, prakash.sangappa, luto, akpm,
oleg, serge.hallyn, esyr, jannh, linux-security-module,
Pavel Emelyanov, Nagarathnam Muthusamy, Eric W. Biederman
In-Reply-To: <87vadmobdw.fsf_-_@xmission.com>
All of the implementations of security hooks that take msg_queue only
access q_perm the struct kern_ipc_perm member. This means the
dependencies of the msg_queue security hooks can be simplified by
passing the kern_ipc_perm member of msg_queue.
Making this change will allow struct msg_queue to become private to
ipc/msg.c.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
include/linux/lsm_hooks.h | 12 ++++++------
include/linux/security.h | 25 ++++++++++++-------------
ipc/msg.c | 18 ++++++++----------
security/security.c | 12 ++++++------
security/selinux/hooks.c | 36 ++++++++++++++++++------------------
security/smack/smack_lsm.c | 24 ++++++++++++------------
6 files changed, 62 insertions(+), 65 deletions(-)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index cac7a8082c43..bde167fa2c51 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1575,13 +1575,13 @@ union security_list_options {
int (*msg_msg_alloc_security)(struct msg_msg *msg);
void (*msg_msg_free_security)(struct msg_msg *msg);
- int (*msg_queue_alloc_security)(struct msg_queue *msq);
- void (*msg_queue_free_security)(struct msg_queue *msq);
- int (*msg_queue_associate)(struct msg_queue *msq, int msqflg);
- int (*msg_queue_msgctl)(struct msg_queue *msq, int cmd);
- int (*msg_queue_msgsnd)(struct msg_queue *msq, struct msg_msg *msg,
+ int (*msg_queue_alloc_security)(struct kern_ipc_perm *msq);
+ void (*msg_queue_free_security)(struct kern_ipc_perm *msq);
+ int (*msg_queue_associate)(struct kern_ipc_perm *msq, int msqflg);
+ int (*msg_queue_msgctl)(struct kern_ipc_perm *msq, int cmd);
+ int (*msg_queue_msgsnd)(struct kern_ipc_perm *msq, struct msg_msg *msg,
int msqflg);
- int (*msg_queue_msgrcv)(struct msg_queue *msq, struct msg_msg *msg,
+ int (*msg_queue_msgrcv)(struct kern_ipc_perm *msq, struct msg_msg *msg,
struct task_struct *target, long type,
int mode);
diff --git a/include/linux/security.h b/include/linux/security.h
index f390755808ea..128e1e4a5346 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -50,7 +50,6 @@ struct iattr;
struct fown_struct;
struct file_operations;
struct msg_msg;
-struct msg_queue;
struct xattr;
struct xfrm_sec_ctx;
struct mm_struct;
@@ -353,13 +352,13 @@ int security_ipc_permission(struct kern_ipc_perm *ipcp, short flag);
void security_ipc_getsecid(struct kern_ipc_perm *ipcp, u32 *secid);
int security_msg_msg_alloc(struct msg_msg *msg);
void security_msg_msg_free(struct msg_msg *msg);
-int security_msg_queue_alloc(struct msg_queue *msq);
-void security_msg_queue_free(struct msg_queue *msq);
-int security_msg_queue_associate(struct msg_queue *msq, int msqflg);
-int security_msg_queue_msgctl(struct msg_queue *msq, int cmd);
-int security_msg_queue_msgsnd(struct msg_queue *msq,
+int security_msg_queue_alloc(struct kern_ipc_perm *msq);
+void security_msg_queue_free(struct kern_ipc_perm *msq);
+int security_msg_queue_associate(struct kern_ipc_perm *msq, int msqflg);
+int security_msg_queue_msgctl(struct kern_ipc_perm *msq, int cmd);
+int security_msg_queue_msgsnd(struct kern_ipc_perm *msq,
struct msg_msg *msg, int msqflg);
-int security_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg *msg,
+int security_msg_queue_msgrcv(struct kern_ipc_perm *msq, struct msg_msg *msg,
struct task_struct *target, long type, int mode);
int security_shm_alloc(struct kern_ipc_perm *shp);
void security_shm_free(struct kern_ipc_perm *shp);
@@ -1043,32 +1042,32 @@ static inline int security_msg_msg_alloc(struct msg_msg *msg)
static inline void security_msg_msg_free(struct msg_msg *msg)
{ }
-static inline int security_msg_queue_alloc(struct msg_queue *msq)
+static inline int security_msg_queue_alloc(struct kern_ipc_perm *msq)
{
return 0;
}
-static inline void security_msg_queue_free(struct msg_queue *msq)
+static inline void security_msg_queue_free(struct kern_ipc_perm *msq)
{ }
-static inline int security_msg_queue_associate(struct msg_queue *msq,
+static inline int security_msg_queue_associate(struct kern_ipc_perm *msq,
int msqflg)
{
return 0;
}
-static inline int security_msg_queue_msgctl(struct msg_queue *msq, int cmd)
+static inline int security_msg_queue_msgctl(struct kern_ipc_perm *msq, int cmd)
{
return 0;
}
-static inline int security_msg_queue_msgsnd(struct msg_queue *msq,
+static inline int security_msg_queue_msgsnd(struct kern_ipc_perm *msq,
struct msg_msg *msg, int msqflg)
{
return 0;
}
-static inline int security_msg_queue_msgrcv(struct msg_queue *msq,
+static inline int security_msg_queue_msgrcv(struct kern_ipc_perm *msq,
struct msg_msg *msg,
struct task_struct *target,
long type, int mode)
diff --git a/ipc/msg.c b/ipc/msg.c
index 0dcc6699dc53..cdfab0825fce 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -101,7 +101,7 @@ static void msg_rcu_free(struct rcu_head *head)
struct kern_ipc_perm *p = container_of(head, struct kern_ipc_perm, rcu);
struct msg_queue *msq = container_of(p, struct msg_queue, q_perm);
- security_msg_queue_free(msq);
+ security_msg_queue_free(&msq->q_perm);
kvfree(msq);
}
@@ -127,7 +127,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
msq->q_perm.key = key;
msq->q_perm.security = NULL;
- retval = security_msg_queue_alloc(msq);
+ retval = security_msg_queue_alloc(&msq->q_perm);
if (retval) {
kvfree(msq);
return retval;
@@ -258,9 +258,7 @@ static void freeque(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp)
*/
static inline int msg_security(struct kern_ipc_perm *ipcp, int msgflg)
{
- struct msg_queue *msq = container_of(ipcp, struct msg_queue, q_perm);
-
- return security_msg_queue_associate(msq, msgflg);
+ return security_msg_queue_associate(ipcp, msgflg);
}
SYSCALL_DEFINE2(msgget, key_t, key, int, msgflg)
@@ -380,7 +378,7 @@ static int msgctl_down(struct ipc_namespace *ns, int msqid, int cmd,
msq = container_of(ipcp, struct msg_queue, q_perm);
- err = security_msg_queue_msgctl(msq, cmd);
+ err = security_msg_queue_msgctl(&msq->q_perm, cmd);
if (err)
goto out_unlock1;
@@ -502,7 +500,7 @@ static int msgctl_stat(struct ipc_namespace *ns, int msqid,
if (ipcperms(ns, &msq->q_perm, S_IRUGO))
goto out_unlock;
- err = security_msg_queue_msgctl(msq, cmd);
+ err = security_msg_queue_msgctl(&msq->q_perm, cmd);
if (err)
goto out_unlock;
@@ -718,7 +716,7 @@ static inline int pipelined_send(struct msg_queue *msq, struct msg_msg *msg,
list_for_each_entry_safe(msr, t, &msq->q_receivers, r_list) {
if (testmsg(msg, msr->r_msgtype, msr->r_mode) &&
- !security_msg_queue_msgrcv(msq, msg, msr->r_tsk,
+ !security_msg_queue_msgrcv(&msq->q_perm, msg, msr->r_tsk,
msr->r_msgtype, msr->r_mode)) {
list_del(&msr->r_list);
@@ -784,7 +782,7 @@ static long do_msgsnd(int msqid, long mtype, void __user *mtext,
goto out_unlock0;
}
- err = security_msg_queue_msgsnd(msq, msg, msgflg);
+ err = security_msg_queue_msgsnd(&msq->q_perm, msg, msgflg);
if (err)
goto out_unlock0;
@@ -960,7 +958,7 @@ static struct msg_msg *find_msg(struct msg_queue *msq, long *msgtyp, int mode)
list_for_each_entry(msg, &msq->q_messages, m_list) {
if (testmsg(msg, *msgtyp, mode) &&
- !security_msg_queue_msgrcv(msq, msg, current,
+ !security_msg_queue_msgrcv(&msq->q_perm, msg, current,
*msgtyp, mode)) {
if (mode == SEARCH_LESSEQUAL && msg->m_type != 1) {
*msgtyp = msg->m_type - 1;
diff --git a/security/security.c b/security/security.c
index 77b69bd6f234..02d734e69955 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1163,33 +1163,33 @@ void security_msg_msg_free(struct msg_msg *msg)
call_void_hook(msg_msg_free_security, msg);
}
-int security_msg_queue_alloc(struct msg_queue *msq)
+int security_msg_queue_alloc(struct kern_ipc_perm *msq)
{
return call_int_hook(msg_queue_alloc_security, 0, msq);
}
-void security_msg_queue_free(struct msg_queue *msq)
+void security_msg_queue_free(struct kern_ipc_perm *msq)
{
call_void_hook(msg_queue_free_security, msq);
}
-int security_msg_queue_associate(struct msg_queue *msq, int msqflg)
+int security_msg_queue_associate(struct kern_ipc_perm *msq, int msqflg)
{
return call_int_hook(msg_queue_associate, 0, msq, msqflg);
}
-int security_msg_queue_msgctl(struct msg_queue *msq, int cmd)
+int security_msg_queue_msgctl(struct kern_ipc_perm *msq, int cmd)
{
return call_int_hook(msg_queue_msgctl, 0, msq, cmd);
}
-int security_msg_queue_msgsnd(struct msg_queue *msq,
+int security_msg_queue_msgsnd(struct kern_ipc_perm *msq,
struct msg_msg *msg, int msqflg)
{
return call_int_hook(msg_queue_msgsnd, 0, msq, msg, msqflg);
}
-int security_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg *msg,
+int security_msg_queue_msgrcv(struct kern_ipc_perm *msq, struct msg_msg *msg,
struct task_struct *target, long type, int mode)
{
return call_int_hook(msg_queue_msgrcv, 0, msq, msg, target, type, mode);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 14f9e6c08273..925e546b5a87 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5532,52 +5532,52 @@ static void selinux_msg_msg_free_security(struct msg_msg *msg)
}
/* message queue security operations */
-static int selinux_msg_queue_alloc_security(struct msg_queue *msq)
+static int selinux_msg_queue_alloc_security(struct kern_ipc_perm *msq)
{
struct ipc_security_struct *isec;
struct common_audit_data ad;
u32 sid = current_sid();
int rc;
- rc = ipc_alloc_security(&msq->q_perm, SECCLASS_MSGQ);
+ rc = ipc_alloc_security(msq, SECCLASS_MSGQ);
if (rc)
return rc;
- isec = msq->q_perm.security;
+ isec = msq->security;
ad.type = LSM_AUDIT_DATA_IPC;
- ad.u.ipc_id = msq->q_perm.key;
+ ad.u.ipc_id = msq->key;
rc = avc_has_perm(sid, isec->sid, SECCLASS_MSGQ,
MSGQ__CREATE, &ad);
if (rc) {
- ipc_free_security(&msq->q_perm);
+ ipc_free_security(msq);
return rc;
}
return 0;
}
-static void selinux_msg_queue_free_security(struct msg_queue *msq)
+static void selinux_msg_queue_free_security(struct kern_ipc_perm *msq)
{
- ipc_free_security(&msq->q_perm);
+ ipc_free_security(msq);
}
-static int selinux_msg_queue_associate(struct msg_queue *msq, int msqflg)
+static int selinux_msg_queue_associate(struct kern_ipc_perm *msq, int msqflg)
{
struct ipc_security_struct *isec;
struct common_audit_data ad;
u32 sid = current_sid();
- isec = msq->q_perm.security;
+ isec = msq->security;
ad.type = LSM_AUDIT_DATA_IPC;
- ad.u.ipc_id = msq->q_perm.key;
+ ad.u.ipc_id = msq->key;
return avc_has_perm(sid, isec->sid, SECCLASS_MSGQ,
MSGQ__ASSOCIATE, &ad);
}
-static int selinux_msg_queue_msgctl(struct msg_queue *msq, int cmd)
+static int selinux_msg_queue_msgctl(struct kern_ipc_perm *msq, int cmd)
{
int err;
int perms;
@@ -5602,11 +5602,11 @@ static int selinux_msg_queue_msgctl(struct msg_queue *msq, int cmd)
return 0;
}
- err = ipc_has_perm(&msq->q_perm, perms);
+ err = ipc_has_perm(msq, perms);
return err;
}
-static int selinux_msg_queue_msgsnd(struct msg_queue *msq, struct msg_msg *msg, int msqflg)
+static int selinux_msg_queue_msgsnd(struct kern_ipc_perm *msq, struct msg_msg *msg, int msqflg)
{
struct ipc_security_struct *isec;
struct msg_security_struct *msec;
@@ -5614,7 +5614,7 @@ static int selinux_msg_queue_msgsnd(struct msg_queue *msq, struct msg_msg *msg,
u32 sid = current_sid();
int rc;
- isec = msq->q_perm.security;
+ isec = msq->security;
msec = msg->security;
/*
@@ -5632,7 +5632,7 @@ static int selinux_msg_queue_msgsnd(struct msg_queue *msq, struct msg_msg *msg,
}
ad.type = LSM_AUDIT_DATA_IPC;
- ad.u.ipc_id = msq->q_perm.key;
+ ad.u.ipc_id = msq->key;
/* Can this process write to the queue? */
rc = avc_has_perm(sid, isec->sid, SECCLASS_MSGQ,
@@ -5649,7 +5649,7 @@ static int selinux_msg_queue_msgsnd(struct msg_queue *msq, struct msg_msg *msg,
return rc;
}
-static int selinux_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg *msg,
+static int selinux_msg_queue_msgrcv(struct kern_ipc_perm *msq, struct msg_msg *msg,
struct task_struct *target,
long type, int mode)
{
@@ -5659,11 +5659,11 @@ static int selinux_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg *msg,
u32 sid = task_sid(target);
int rc;
- isec = msq->q_perm.security;
+ isec = msq->security;
msec = msg->security;
ad.type = LSM_AUDIT_DATA_IPC;
- ad.u.ipc_id = msq->q_perm.key;
+ ad.u.ipc_id = msq->key;
rc = avc_has_perm(sid, isec->sid,
SECCLASS_MSGQ, MSGQ__READ, &ad);
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index a3398c7f32c9..d960c2ea8d79 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3210,9 +3210,9 @@ static int smack_sem_semop(struct kern_ipc_perm *sma, struct sembuf *sops,
*
* Returns 0
*/
-static int smack_msg_queue_alloc_security(struct msg_queue *msq)
+static int smack_msg_queue_alloc_security(struct kern_ipc_perm *msq)
{
- struct kern_ipc_perm *kisp = &msq->q_perm;
+ struct kern_ipc_perm *kisp = msq;
struct smack_known *skp = smk_of_current();
kisp->security = skp;
@@ -3225,9 +3225,9 @@ static int smack_msg_queue_alloc_security(struct msg_queue *msq)
*
* Clears the blob pointer
*/
-static void smack_msg_queue_free_security(struct msg_queue *msq)
+static void smack_msg_queue_free_security(struct kern_ipc_perm *msq)
{
- struct kern_ipc_perm *kisp = &msq->q_perm;
+ struct kern_ipc_perm *kisp = msq;
kisp->security = NULL;
}
@@ -3238,9 +3238,9 @@ static void smack_msg_queue_free_security(struct msg_queue *msq)
*
* Returns a pointer to the smack label entry
*/
-static struct smack_known *smack_of_msq(struct msg_queue *msq)
+static struct smack_known *smack_of_msq(struct kern_ipc_perm *msq)
{
- return (struct smack_known *)msq->q_perm.security;
+ return (struct smack_known *)msq->security;
}
/**
@@ -3250,7 +3250,7 @@ static struct smack_known *smack_of_msq(struct msg_queue *msq)
*
* return 0 if current has access, error otherwise
*/
-static int smk_curacc_msq(struct msg_queue *msq, int access)
+static int smk_curacc_msq(struct kern_ipc_perm *msq, int access)
{
struct smack_known *msp = smack_of_msq(msq);
struct smk_audit_info ad;
@@ -3258,7 +3258,7 @@ static int smk_curacc_msq(struct msg_queue *msq, int access)
#ifdef CONFIG_AUDIT
smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_IPC);
- ad.a.u.ipc_id = msq->q_perm.id;
+ ad.a.u.ipc_id = msq->id;
#endif
rc = smk_curacc(msp, access, &ad);
rc = smk_bu_current("msq", msp, access, rc);
@@ -3272,7 +3272,7 @@ static int smk_curacc_msq(struct msg_queue *msq, int access)
*
* Returns 0 if current has the requested access, error code otherwise
*/
-static int smack_msg_queue_associate(struct msg_queue *msq, int msqflg)
+static int smack_msg_queue_associate(struct kern_ipc_perm *msq, int msqflg)
{
int may;
@@ -3287,7 +3287,7 @@ static int smack_msg_queue_associate(struct msg_queue *msq, int msqflg)
*
* Returns 0 if current has the requested access, error code otherwise
*/
-static int smack_msg_queue_msgctl(struct msg_queue *msq, int cmd)
+static int smack_msg_queue_msgctl(struct kern_ipc_perm *msq, int cmd)
{
int may;
@@ -3321,7 +3321,7 @@ static int smack_msg_queue_msgctl(struct msg_queue *msq, int cmd)
*
* Returns 0 if current has the requested access, error code otherwise
*/
-static int smack_msg_queue_msgsnd(struct msg_queue *msq, struct msg_msg *msg,
+static int smack_msg_queue_msgsnd(struct kern_ipc_perm *msq, struct msg_msg *msg,
int msqflg)
{
int may;
@@ -3340,7 +3340,7 @@ static int smack_msg_queue_msgsnd(struct msg_queue *msq, struct msg_msg *msg,
*
* Returns 0 if current has read and write access, error code otherwise
*/
-static int smack_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg *msg,
+static int smack_msg_queue_msgrcv(struct kern_ipc_perm *msq, struct msg_msg *msg,
struct task_struct *target, long type, int mode)
{
return smk_curacc_msq(msq, MAY_READWRITE);
--
2.14.1
^ permalink raw reply related
* [REVIEW][PATCH 02/11] shm/security: Pass kern_ipc_perm not shmid_kernel into the shm security hooks
From: Eric W. Biederman @ 2018-03-23 19:16 UTC (permalink / raw)
To: Linux Containers
Cc: esyr-H+wXaHxf7aLQT0dZR+AlfA, jannh-hpIqsD4AKlfQT0dZR+AlfA,
khlebnikov-XoJtRXgx1JseBXzfvpsJ4g,
linux-api-u79uwXL29TY76Z2rM5mHXA,
serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
prakash.sangappa-QHcLZuEGTsvQT0dZR+AlfA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA, Eric W. Biederman,
luto-DgEjT+Ai2ygdnm+yROfE0A, oleg-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Nagarathnam Muthusamy,
Pavel Emelyanov
In-Reply-To: <87vadmobdw.fsf_-_-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
All of the implementations of security hooks that take shmid_kernel only
access shm_perm the struct kern_ipc_perm member. This means the
dependencies of the shm security hooks can be simplified by passing
the kern_ipc_perm member of shmid_kernel..
Making this change will allow struct shmid_kernel to become private to ipc/shm.c.
Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
include/linux/lsm_hooks.h | 10 +++++-----
include/linux/security.h | 21 ++++++++++-----------
ipc/shm.c | 17 +++++++----------
security/security.c | 10 +++++-----
security/selinux/hooks.c | 28 ++++++++++++++--------------
security/smack/smack_lsm.c | 22 +++++++++++-----------
6 files changed, 52 insertions(+), 56 deletions(-)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index e4a94863a88c..cac7a8082c43 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1585,11 +1585,11 @@ union security_list_options {
struct task_struct *target, long type,
int mode);
- int (*shm_alloc_security)(struct shmid_kernel *shp);
- void (*shm_free_security)(struct shmid_kernel *shp);
- int (*shm_associate)(struct shmid_kernel *shp, int shmflg);
- int (*shm_shmctl)(struct shmid_kernel *shp, int cmd);
- int (*shm_shmat)(struct shmid_kernel *shp, char __user *shmaddr,
+ int (*shm_alloc_security)(struct kern_ipc_perm *shp);
+ void (*shm_free_security)(struct kern_ipc_perm *shp);
+ int (*shm_associate)(struct kern_ipc_perm *shp, int shmflg);
+ int (*shm_shmctl)(struct kern_ipc_perm *shp, int cmd);
+ int (*shm_shmat)(struct kern_ipc_perm *shp, char __user *shmaddr,
int shmflg);
int (*sem_alloc_security)(struct kern_ipc_perm *sma);
diff --git a/include/linux/security.h b/include/linux/security.h
index fa7adac4b99a..f390755808ea 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -49,7 +49,6 @@ struct qstr;
struct iattr;
struct fown_struct;
struct file_operations;
-struct shmid_kernel;
struct msg_msg;
struct msg_queue;
struct xattr;
@@ -362,11 +361,11 @@ int security_msg_queue_msgsnd(struct msg_queue *msq,
struct msg_msg *msg, int msqflg);
int security_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg *msg,
struct task_struct *target, long type, int mode);
-int security_shm_alloc(struct shmid_kernel *shp);
-void security_shm_free(struct shmid_kernel *shp);
-int security_shm_associate(struct shmid_kernel *shp, int shmflg);
-int security_shm_shmctl(struct shmid_kernel *shp, int cmd);
-int security_shm_shmat(struct shmid_kernel *shp, char __user *shmaddr, int shmflg);
+int security_shm_alloc(struct kern_ipc_perm *shp);
+void security_shm_free(struct kern_ipc_perm *shp);
+int security_shm_associate(struct kern_ipc_perm *shp, int shmflg);
+int security_shm_shmctl(struct kern_ipc_perm *shp, int cmd);
+int security_shm_shmat(struct kern_ipc_perm *shp, char __user *shmaddr, int shmflg);
int security_sem_alloc(struct kern_ipc_perm *sma);
void security_sem_free(struct kern_ipc_perm *sma);
int security_sem_associate(struct kern_ipc_perm *sma, int semflg);
@@ -1077,26 +1076,26 @@ static inline int security_msg_queue_msgrcv(struct msg_queue *msq,
return 0;
}
-static inline int security_shm_alloc(struct shmid_kernel *shp)
+static inline int security_shm_alloc(struct kern_ipc_perm *shp)
{
return 0;
}
-static inline void security_shm_free(struct shmid_kernel *shp)
+static inline void security_shm_free(struct kern_ipc_perm *shp)
{ }
-static inline int security_shm_associate(struct shmid_kernel *shp,
+static inline int security_shm_associate(struct kern_ipc_perm *shp,
int shmflg)
{
return 0;
}
-static inline int security_shm_shmctl(struct shmid_kernel *shp, int cmd)
+static inline int security_shm_shmctl(struct kern_ipc_perm *shp, int cmd)
{
return 0;
}
-static inline int security_shm_shmat(struct shmid_kernel *shp,
+static inline int security_shm_shmat(struct kern_ipc_perm *shp,
char __user *shmaddr, int shmflg)
{
return 0;
diff --git a/ipc/shm.c b/ipc/shm.c
index 4643865e9171..387a786e7be1 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -181,7 +181,7 @@ static void shm_rcu_free(struct rcu_head *head)
rcu);
struct shmid_kernel *shp = container_of(ptr, struct shmid_kernel,
shm_perm);
- security_shm_free(shp);
+ security_shm_free(&shp->shm_perm);
kvfree(shp);
}
@@ -554,7 +554,7 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
shp->mlock_user = NULL;
shp->shm_perm.security = NULL;
- error = security_shm_alloc(shp);
+ error = security_shm_alloc(&shp->shm_perm);
if (error) {
kvfree(shp);
return error;
@@ -635,10 +635,7 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
*/
static inline int shm_security(struct kern_ipc_perm *ipcp, int shmflg)
{
- struct shmid_kernel *shp;
-
- shp = container_of(ipcp, struct shmid_kernel, shm_perm);
- return security_shm_associate(shp, shmflg);
+ return security_shm_associate(ipcp, shmflg);
}
/*
@@ -835,7 +832,7 @@ static int shmctl_down(struct ipc_namespace *ns, int shmid, int cmd,
shp = container_of(ipcp, struct shmid_kernel, shm_perm);
- err = security_shm_shmctl(shp, cmd);
+ err = security_shm_shmctl(&shp->shm_perm, cmd);
if (err)
goto out_unlock1;
@@ -934,7 +931,7 @@ static int shmctl_stat(struct ipc_namespace *ns, int shmid,
if (ipcperms(ns, &shp->shm_perm, S_IRUGO))
goto out_unlock;
- err = security_shm_shmctl(shp, cmd);
+ err = security_shm_shmctl(&shp->shm_perm, cmd);
if (err)
goto out_unlock;
@@ -978,7 +975,7 @@ static int shmctl_do_lock(struct ipc_namespace *ns, int shmid, int cmd)
}
audit_ipc_obj(&(shp->shm_perm));
- err = security_shm_shmctl(shp, cmd);
+ err = security_shm_shmctl(&shp->shm_perm, cmd);
if (err)
goto out_unlock1;
@@ -1348,7 +1345,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg,
if (ipcperms(ns, &shp->shm_perm, acc_mode))
goto out_unlock;
- err = security_shm_shmat(shp, shmaddr, shmflg);
+ err = security_shm_shmat(&shp->shm_perm, shmaddr, shmflg);
if (err)
goto out_unlock;
diff --git a/security/security.c b/security/security.c
index d3b9aeb6b73b..77b69bd6f234 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1195,27 +1195,27 @@ int security_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg *msg,
return call_int_hook(msg_queue_msgrcv, 0, msq, msg, target, type, mode);
}
-int security_shm_alloc(struct shmid_kernel *shp)
+int security_shm_alloc(struct kern_ipc_perm *shp)
{
return call_int_hook(shm_alloc_security, 0, shp);
}
-void security_shm_free(struct shmid_kernel *shp)
+void security_shm_free(struct kern_ipc_perm *shp)
{
call_void_hook(shm_free_security, shp);
}
-int security_shm_associate(struct shmid_kernel *shp, int shmflg)
+int security_shm_associate(struct kern_ipc_perm *shp, int shmflg)
{
return call_int_hook(shm_associate, 0, shp, shmflg);
}
-int security_shm_shmctl(struct shmid_kernel *shp, int cmd)
+int security_shm_shmctl(struct kern_ipc_perm *shp, int cmd)
{
return call_int_hook(shm_shmctl, 0, shp, cmd);
}
-int security_shm_shmat(struct shmid_kernel *shp, char __user *shmaddr, int shmflg)
+int security_shm_shmat(struct kern_ipc_perm *shp, char __user *shmaddr, int shmflg)
{
return call_int_hook(shm_shmat, 0, shp, shmaddr, shmflg);
}
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index cce994e9fc0a..14f9e6c08273 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5674,53 +5674,53 @@ static int selinux_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg *msg,
}
/* Shared Memory security operations */
-static int selinux_shm_alloc_security(struct shmid_kernel *shp)
+static int selinux_shm_alloc_security(struct kern_ipc_perm *shp)
{
struct ipc_security_struct *isec;
struct common_audit_data ad;
u32 sid = current_sid();
int rc;
- rc = ipc_alloc_security(&shp->shm_perm, SECCLASS_SHM);
+ rc = ipc_alloc_security(shp, SECCLASS_SHM);
if (rc)
return rc;
- isec = shp->shm_perm.security;
+ isec = shp->security;
ad.type = LSM_AUDIT_DATA_IPC;
- ad.u.ipc_id = shp->shm_perm.key;
+ ad.u.ipc_id = shp->key;
rc = avc_has_perm(sid, isec->sid, SECCLASS_SHM,
SHM__CREATE, &ad);
if (rc) {
- ipc_free_security(&shp->shm_perm);
+ ipc_free_security(shp);
return rc;
}
return 0;
}
-static void selinux_shm_free_security(struct shmid_kernel *shp)
+static void selinux_shm_free_security(struct kern_ipc_perm *shp)
{
- ipc_free_security(&shp->shm_perm);
+ ipc_free_security(shp);
}
-static int selinux_shm_associate(struct shmid_kernel *shp, int shmflg)
+static int selinux_shm_associate(struct kern_ipc_perm *shp, int shmflg)
{
struct ipc_security_struct *isec;
struct common_audit_data ad;
u32 sid = current_sid();
- isec = shp->shm_perm.security;
+ isec = shp->security;
ad.type = LSM_AUDIT_DATA_IPC;
- ad.u.ipc_id = shp->shm_perm.key;
+ ad.u.ipc_id = shp->key;
return avc_has_perm(sid, isec->sid, SECCLASS_SHM,
SHM__ASSOCIATE, &ad);
}
/* Note, at this point, shp is locked down */
-static int selinux_shm_shmctl(struct shmid_kernel *shp, int cmd)
+static int selinux_shm_shmctl(struct kern_ipc_perm *shp, int cmd)
{
int perms;
int err;
@@ -5749,11 +5749,11 @@ static int selinux_shm_shmctl(struct shmid_kernel *shp, int cmd)
return 0;
}
- err = ipc_has_perm(&shp->shm_perm, perms);
+ err = ipc_has_perm(shp, perms);
return err;
}
-static int selinux_shm_shmat(struct shmid_kernel *shp,
+static int selinux_shm_shmat(struct kern_ipc_perm *shp,
char __user *shmaddr, int shmflg)
{
u32 perms;
@@ -5763,7 +5763,7 @@ static int selinux_shm_shmat(struct shmid_kernel *shp,
else
perms = SHM__READ | SHM__WRITE;
- return ipc_has_perm(&shp->shm_perm, perms);
+ return ipc_has_perm(shp, perms);
}
/* Semaphore security operations */
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 0402b8c1aec1..a3398c7f32c9 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2950,9 +2950,9 @@ static void smack_msg_msg_free_security(struct msg_msg *msg)
*
* Returns a pointer to the smack value
*/
-static struct smack_known *smack_of_shm(struct shmid_kernel *shp)
+static struct smack_known *smack_of_shm(struct kern_ipc_perm *shp)
{
- return (struct smack_known *)shp->shm_perm.security;
+ return (struct smack_known *)shp->security;
}
/**
@@ -2961,9 +2961,9 @@ static struct smack_known *smack_of_shm(struct shmid_kernel *shp)
*
* Returns 0
*/
-static int smack_shm_alloc_security(struct shmid_kernel *shp)
+static int smack_shm_alloc_security(struct kern_ipc_perm *shp)
{
- struct kern_ipc_perm *isp = &shp->shm_perm;
+ struct kern_ipc_perm *isp = shp;
struct smack_known *skp = smk_of_current();
isp->security = skp;
@@ -2976,9 +2976,9 @@ static int smack_shm_alloc_security(struct shmid_kernel *shp)
*
* Clears the blob pointer
*/
-static void smack_shm_free_security(struct shmid_kernel *shp)
+static void smack_shm_free_security(struct kern_ipc_perm *shp)
{
- struct kern_ipc_perm *isp = &shp->shm_perm;
+ struct kern_ipc_perm *isp = shp;
isp->security = NULL;
}
@@ -2990,7 +2990,7 @@ static void smack_shm_free_security(struct shmid_kernel *shp)
*
* Returns 0 if current has the requested access, error code otherwise
*/
-static int smk_curacc_shm(struct shmid_kernel *shp, int access)
+static int smk_curacc_shm(struct kern_ipc_perm *shp, int access)
{
struct smack_known *ssp = smack_of_shm(shp);
struct smk_audit_info ad;
@@ -2998,7 +2998,7 @@ static int smk_curacc_shm(struct shmid_kernel *shp, int access)
#ifdef CONFIG_AUDIT
smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_IPC);
- ad.a.u.ipc_id = shp->shm_perm.id;
+ ad.a.u.ipc_id = shp->id;
#endif
rc = smk_curacc(ssp, access, &ad);
rc = smk_bu_current("shm", ssp, access, rc);
@@ -3012,7 +3012,7 @@ static int smk_curacc_shm(struct shmid_kernel *shp, int access)
*
* Returns 0 if current has the requested access, error code otherwise
*/
-static int smack_shm_associate(struct shmid_kernel *shp, int shmflg)
+static int smack_shm_associate(struct kern_ipc_perm *shp, int shmflg)
{
int may;
@@ -3027,7 +3027,7 @@ static int smack_shm_associate(struct shmid_kernel *shp, int shmflg)
*
* Returns 0 if current has the requested access, error code otherwise
*/
-static int smack_shm_shmctl(struct shmid_kernel *shp, int cmd)
+static int smack_shm_shmctl(struct kern_ipc_perm *shp, int cmd)
{
int may;
@@ -3062,7 +3062,7 @@ static int smack_shm_shmctl(struct shmid_kernel *shp, int cmd)
*
* Returns 0 if current has the requested access, error code otherwise
*/
-static int smack_shm_shmat(struct shmid_kernel *shp, char __user *shmaddr,
+static int smack_shm_shmat(struct kern_ipc_perm *shp, char __user *shmaddr,
int shmflg)
{
int may;
--
2.14.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox