Re: [PATCH v3 net-next 3/5] sock: ULP infrastructure
On Tue, Aug 8, 2017 at 9:38 AM, Hannes Frederic Sowa wrote: > Tom Herbert writes: > >> +#ifdef CONFIG_MODULES >> + if (!ulp && capable(CAP_NET_ADMIN)) { >> + rcu_read_unlock(); >> + request_module("%s", name); >> + rcu_read_lock(); >> + ulp = ulp_find(name); >> + } >> +#endif > > It looks to me that this allows users with only CAP_NET_ADMIN > privileges to load every module? It's a carryover. Probably should remove the check. Tom
Re: [PATCH v3 net-next 3/5] sock: ULP infrastructure
Tom Herbert writes: > +#ifdef CONFIG_MODULES > + if (!ulp && capable(CAP_NET_ADMIN)) { > + rcu_read_unlock(); > + request_module("%s", name); > + rcu_read_lock(); > + ulp = ulp_find(name); > + } > +#endif It looks to me that this allows users with only CAP_NET_ADMIN privileges to load every module?
Re: [PATCH v3 net-next 3/5] sock: ULP infrastructure
On 08/07/2017 10:28 AM, Tom Herbert wrote: > Generalize the TCP ULP infrastructure recently introduced to support > kTLS. This adds a SO_ULP socket option and creates new fields in > sock structure for ULP ops and ULP data. Also, the interface allows > additional per ULP parameters to be set so that a ULP can be pushed > and operations started in one shot. > > Signed-off-by: Tom Herbert > --- I think this generalization should not get committed until it has a user. I see you posted the socktap stuff but that is just an RFC for now. [...] > + > +static inline void ulp_get_available(char *buf, size_t len) > +{ Do we need to check len field or is len == 0 invalid? > + *buf = '\0'; > +} > + > +static inline void ulp_cleanup(struct sock *sk) > +{ > +} > + [...] Rest looks OK I'll take a closer look tomorrow at this and the RFC user. Thanks, John