mbox series

[0/3] net/sched: Load modules via alias

Message ID 20231206192752.18989-1-mkoutny@suse.com (mailing list archive)
Headers show
Series net/sched: Load modules via alias | expand

Message

Michal Koutný Dec. 6, 2023, 7:27 p.m. UTC
These modules may be loaded lazily without user's awareness and
control. Add respective aliases to modules and request them under these
aliases so that modprobe's blacklisting mechanism (through aliases)
works for them. (The same pattern exists e.g. for filesystem
modules.)

Original module names remain unchanged.

Patches were generated with the help of Coccinelle scripts like:

cat >scripts/coccinelle/misc/tcf_alias.cocci <<EOD
virtual patch
virtual report

@ haskernel @
@@

@ tcf_has_kind depends on report && haskernel @
identifier ops;
constant K;
@@

  static struct tcf_proto_ops ops = {
    .kind = K,
    ...
  };
+char module_alias = K;
EOD

/usr/bin/spatch -D report --cocci-file scripts/coccinelle/misc/tcf_alias.cocci \
	--dir . \
	-I ./arch/x86/include -I ./arch/x86/include/generated -I ./include \
	-I ./arch/x86/include/uapi -I ./arch/x86/include/generated/uapi \
	-I ./include/uapi -I ./include/generated/uapi \
	--include ./include/linux/compiler-version.h --include ./include/linux/kconfig.h \
	--jobs 8 --chunksize 1 2>/dev/null | \
	sed 's/char module_alias = "\([^"]*\)";/MODULE_ALIAS_TCF("\1");/'

Changes from v1 (https://lore.kernel.org/r/20231121175640.9981-1-mkoutny@suse.com)
- Treat sch_ and act_ modules analogously to cls_

Michal Koutný (3):
  net/sched: cls: Load net classifier modules via alias
  net/sched: sch: Load qdisc modules via alias
  net/sched: act: Load TC action modules via alias

 include/net/act_api.h      | 1 +
 include/net/pkt_cls.h      | 1 +
 include/net/pkt_sched.h    | 1 +
 net/sched/act_api.c        | 2 +-
 net/sched/act_bpf.c        | 1 +
 net/sched/act_connmark.c   | 1 +
 net/sched/act_csum.c       | 1 +
 net/sched/act_ct.c         | 1 +
 net/sched/act_ctinfo.c     | 1 +
 net/sched/act_gact.c       | 1 +
 net/sched/act_gate.c       | 1 +
 net/sched/act_ife.c        | 1 +
 net/sched/act_ipt.c        | 2 ++
 net/sched/act_mirred.c     | 1 +
 net/sched/act_mpls.c       | 1 +
 net/sched/act_nat.c        | 1 +
 net/sched/act_pedit.c      | 1 +
 net/sched/act_police.c     | 1 +
 net/sched/act_sample.c     | 1 +
 net/sched/act_simple.c     | 1 +
 net/sched/act_skbedit.c    | 1 +
 net/sched/act_skbmod.c     | 1 +
 net/sched/act_tunnel_key.c | 1 +
 net/sched/act_vlan.c       | 1 +
 net/sched/cls_api.c        | 2 +-
 net/sched/cls_basic.c      | 1 +
 net/sched/cls_bpf.c        | 1 +
 net/sched/cls_cgroup.c     | 1 +
 net/sched/cls_flow.c       | 1 +
 net/sched/cls_flower.c     | 1 +
 net/sched/cls_fw.c         | 1 +
 net/sched/cls_matchall.c   | 1 +
 net/sched/cls_route.c      | 1 +
 net/sched/cls_u32.c        | 1 +
 net/sched/sch_api.c        | 2 +-
 net/sched/sch_cake.c       | 1 +
 net/sched/sch_cbs.c        | 1 +
 net/sched/sch_choke.c      | 1 +
 net/sched/sch_codel.c      | 1 +
 net/sched/sch_drr.c        | 1 +
 net/sched/sch_etf.c        | 1 +
 net/sched/sch_ets.c        | 1 +
 net/sched/sch_fq.c         | 1 +
 net/sched/sch_fq_codel.c   | 1 +
 net/sched/sch_gred.c       | 1 +
 net/sched/sch_hfsc.c       | 1 +
 net/sched/sch_hhf.c        | 1 +
 net/sched/sch_htb.c        | 1 +
 net/sched/sch_ingress.c    | 2 ++
 net/sched/sch_mqprio.c     | 1 +
 net/sched/sch_multiq.c     | 1 +
 net/sched/sch_netem.c      | 1 +
 net/sched/sch_pie.c        | 1 +
 net/sched/sch_plug.c       | 1 +
 net/sched/sch_prio.c       | 1 +
 net/sched/sch_qfq.c        | 1 +
 net/sched/sch_red.c        | 1 +
 net/sched/sch_sfb.c        | 1 +
 net/sched/sch_sfq.c        | 1 +
 net/sched/sch_skbprio.c    | 1 +
 net/sched/sch_taprio.c     | 1 +
 net/sched/sch_tbf.c        | 1 +
 62 files changed, 64 insertions(+), 3 deletions(-)

Comments

Pedro Tammela Dec. 6, 2023, 8:16 p.m. UTC | #1
On 06/12/2023 16:27, Michal Koutný wrote:
> These modules may be loaded lazily without user's awareness and
> control. Add respective aliases to modules and request them under these
> aliases so that modprobe's blacklisting mechanism (through aliases)
> works for them. (The same pattern exists e.g. for filesystem
> modules.)
> 
> Original module names remain unchanged.
> 

Can't you just keep the sch-, cls-, act- prefixes for the aliases?
They look odd in the current patchset TBH
Michal Koutný Dec. 6, 2023, 9:18 p.m. UTC | #2
On Wed, Dec 06, 2023 at 05:16:28PM -0300, Pedro Tammela <pctammela@mojatatu.com> wrote:
> Can't you just keep the sch-, cls-, act- prefixes for the aliases?
> They look odd in the current patchset TBH

I'm open to different better naming.

Although, this natural option would clash with the behavior
(modprobe(8)):

> there is no difference between _ and - in module names

Thus blacklisting via an alias vs not-blacklisting via non-canonical
name would contradict each other :-/

Michal
Stephen Hemminger Dec. 6, 2023, 10:28 p.m. UTC | #3
On Wed, 6 Dec 2023 22:18:25 +0100
Michal Koutný <mkoutny@suse.com> wrote:

> On Wed, Dec 06, 2023 at 05:16:28PM -0300, Pedro Tammela <pctammela@mojatatu.com> wrote:
> > Can't you just keep the sch-, cls-, act- prefixes for the aliases?
> > They look odd in the current patchset TBH  
> 
> I'm open to different better naming.
> 
> Although, this natural option would clash with the behavior
> (modprobe(8)):
> 
> > there is no difference between _ and - in module names  
> 
> Thus blacklisting via an alias vs not-blacklisting via non-canonical
> name would contradict each other :-/
> 
> Michal

It is not clear to me what this patchset is trying to fix.
Autoloading happens now, but it does depend on the name not alias.
Michal Koutný Dec. 6, 2023, 10:49 p.m. UTC | #4
On Wed, Dec 06, 2023 at 02:28:57PM -0800, Stephen Hemminger <stephen@networkplumber.org> wrote:
> It is not clear to me what this patchset is trying to fix.
> Autoloading happens now, but it does depend on the name not alias.

There are some more details in the thread of v1 [1] [2].
Does it clarify?

Thanks,
Michal

[1] https://lore.kernel.org/r/yerqczxbz6qlrslkfbu6u2emb5esqe7tkrexdbneite2ah2a6i@l6arp7nzyj75/
[2] Oh, I realize I forgot to add v2 to today's posting.
Toke Høiland-Jørgensen Dec. 6, 2023, 11:42 p.m. UTC | #5
Michal Koutný <mkoutny@suse.com> writes:

> On Wed, Dec 06, 2023 at 02:28:57PM -0800, Stephen Hemminger <stephen@networkplumber.org> wrote:
>> It is not clear to me what this patchset is trying to fix.
>> Autoloading happens now, but it does depend on the name not alias.
>
> There are some more details in the thread of v1 [1] [2].
> Does it clarify?

Yes, but this should be explained clearly in the commit message
(including the reason why this is useful, in the follow-up to [1]).

-Toke
Stephen Hemminger Dec. 6, 2023, 11:55 p.m. UTC | #6
On Wed, 6 Dec 2023 23:49:14 +0100
Michal Koutný <mkoutny@suse.com> wrote:

> On Wed, Dec 06, 2023 at 02:28:57PM -0800, Stephen Hemminger <stephen@networkplumber.org> wrote:
> > It is not clear to me what this patchset is trying to fix.
> > Autoloading happens now, but it does depend on the name not alias.  
> 
> There are some more details in the thread of v1 [1] [2].
> Does it clarify?
> 
> Thanks,
> Michal
> 
> [1] https://lore.kernel.org/r/yerqczxbz6qlrslkfbu6u2emb5esqe7tkrexdbneite2ah2a6i@l6arp7nzyj75/
> [2] Oh, I realize I forgot to add v2 to today's posting.
> 
> 

So your using blacklist as workaround security method and the name confuses it now.