From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christian Brauner Subject: Re: [PATCH net-next 1/2 v3] uevent: add alloc_uevent_skb() helper Date: Sat, 28 Apr 2018 21:09:45 +0200 Message-ID: <20180428190944.GA13787@gmail.com> References: <20180427102306.8617-1-christian.brauner@ubuntu.com> <20180427102306.8617-2-christian.brauner@ubuntu.com> <878t987gdr.fsf@xmission.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Cc: davem@davemloft.net, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, avagin@virtuozzo.com, ktkhai@virtuozzo.com, serge@hallyn.com, gregkh@linuxfoundation.org To: "Eric W. Biederman" Return-path: Content-Disposition: inline In-Reply-To: <878t987gdr.fsf@xmission.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Fri, Apr 27, 2018 at 11:39:44AM -0500, Eric W. Biederman wrote: > Christian Brauner writes: > > > This patch adds alloc_uevent_skb() in preparation for follow up patches. > > > > Signed-off-by: Christian Brauner > > --- > > lib/kobject_uevent.c | 39 ++++++++++++++++++++++++++------------- > > 1 file changed, 26 insertions(+), 13 deletions(-) > > > > diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c > > index 15ea216a67ce..c3cb110f663b 100644 > > --- a/lib/kobject_uevent.c > > +++ b/lib/kobject_uevent.c > > @@ -296,6 +296,31 @@ static void cleanup_uevent_env(struct subprocess_info *info) > > } > > #endif > > > > +static struct sk_buff *alloc_uevent_skb(struct kobj_uevent_env *env, > > + const char *action_string, > > + const char *devpath) > > +{ > > + struct sk_buff *skb = NULL; > > + char *scratch; > > + size_t len; > > + > > + /* allocate message with maximum possible size */ > > + len = strlen(action_string) + strlen(devpath) + 2; > > + skb = alloc_skb(len + env->buflen, GFP_KERNEL); > > + if (!skb) > > + return NULL; > > + > > + /* add header */ > > + scratch = skb_put(skb, len); > > + sprintf(scratch, "%s@%s", action_string, devpath); > > + > > + skb_put_data(skb, env->buf, env->buflen); > > + > > + NETLINK_CB(skb).dst_group = 1; > > nit: > We might want to explicitly set NETLINK_CB(skb).portid to 0 and > NETLINK_CB(skb).creds.uid to GLOBAL_ROOT_UID and > NETLINK_CB(skb).creds.gid to GLOBAL_ROOT_GID here > just to make it clear this is happening. > > It is not a problem because they __alloc_skb memsets to 0 the > fields of struct sk_buff that it does not initialize. And these > are the zero values. > > Still it would be nice to be able to look at the code and quickly > see these are the values being set. Don't really mind adding it. Ok, non-functional changes added to the new version. But then let's set "portid" too: parms = &NETLINK_CB(skb); parms->creds.uid = GLOBAL_ROOT_UID; parms->creds.gid = GLOBAL_ROOT_GID; parms->dst_group = 1; parms->portid = 0; Christian