Linux-Modules Archive on lore.kernel.org
 help / color / Atom feed
From: Lucas De Marchi <lucas.de.marchi@gmail.com>
To: Harald Hoyer <harald@redhat.com>
Cc: linux-modules@vger.kernel.org, Jean Delvare <jdelvare@suse.de>,
	Pavel Shilovsky <pshilov@microsoft.com>,
	linux-cifs@vger.kernel.org
Subject: Re: multiple softdeps
Date: Fri, 28 Jun 2019 09:25:17 -0700
Message-ID: <20190628162517.GA24484@ldmartin-desk1> (raw)
In-Reply-To: <0bfb6f60-0042-f6be-24ed-7803b6ac759c@redhat.com>

+cifs, +Jean, +Pavel

On Fri, Jun 28, 2019 at 03:16:35PM +0200, Harald Hoyer wrote:
>Hi,
>
>could you please enlighten me about kernel module softdeps?
>
>$ modinfo cifs | grep soft
>softdep:        pre: ccm
>softdep:        pre: aead2
>softdep:        pre: sha512
>softdep:        pre: sha256
>softdep:        pre: cmac
>softdep:        pre: aes
>softdep:        pre: nls
>softdep:        pre: md5
>softdep:        pre: md4
>softdep:        pre: hmac
>softdep:        pre: ecb
>softdep:        pre: des
>softdep:        pre: arc4
>
>$ grep cifs /lib/modules/$(uname -r)/modules.softdep
>softdep cifs pre: ccm
>softdep cifs pre: aead2
>softdep cifs pre: sha512
>softdep cifs pre: sha256
>softdep cifs pre: cmac
>softdep cifs pre: aes
>softdep cifs pre: nls
>softdep cifs pre: md5
>softdep cifs pre: md4
>softdep cifs pre: hmac
>softdep cifs pre: ecb
>softdep cifs pre: des
>softdep cifs pre: arc4

this is your bug. Multiple softdeps are not additive to the previous
configuration, we never supported that. Commit b9be76d585d4 ("cifs: Add
soft dependencies") added it for the wrong reasons actually. A sotfdep
means kmod will actually load those dependencies before loading the
module (or fail to load it if those dependencies don't exist).
Besides the wrong commit message, if that is indeed what is desired, the
fix would be:

--------8<--------------
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 65d9771e49f9..4f1f744ea3cd 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -1591,18 +1591,6 @@ MODULE_DESCRIPTION
 	("VFS to access SMB3 servers e.g. Samba, Macs, Azure and Windows (and "
 	"also older servers complying with the SNIA CIFS Specification)");
 MODULE_VERSION(CIFS_VERSION);
-MODULE_SOFTDEP("pre: arc4");
-MODULE_SOFTDEP("pre: des");
-MODULE_SOFTDEP("pre: ecb");
-MODULE_SOFTDEP("pre: hmac");
-MODULE_SOFTDEP("pre: md4");
-MODULE_SOFTDEP("pre: md5");
-MODULE_SOFTDEP("pre: nls");
-MODULE_SOFTDEP("pre: aes");
-MODULE_SOFTDEP("pre: cmac");
-MODULE_SOFTDEP("pre: sha256");
-MODULE_SOFTDEP("pre: sha512");
-MODULE_SOFTDEP("pre: aead2");
-MODULE_SOFTDEP("pre: ccm");
+MODULE_SOFTDEP("pre: arc4 des ecb hmac md4 md5 nls aes cmac sha256 sha512 aead2 ccm");
 module_init(init_cifs)
 module_exit(exit_cifs)
--------8<--------------

I guess we could actually implement additive deps, e.g. by using
"pre+:" or something else. But I think the right thing to do for now is
to apply something like above to cifs and propagate it to stable.

Thanks for the report.

Lucas De Marchi

>
>But, calling kmod_module_get_softdeps() on the cifs module only returns one module in the pre list ("ccm").
>
>Is my understanding about how softdeps work wrong, or is the cifs module misconfigured, or is libkmod buggy?
>Please CC me, as I am not subscribed to the mailing list.
>
>
>softdeps-test.c:
>
>#include <stdio.h>
>#include <libkmod.h>
>#include <stdlib.h>
>
>int main() {
>    int err;
>    struct kmod_ctx *ctx = NULL;
>    struct kmod_list *list = NULL;
>    struct kmod_list *modpre = NULL;
>    struct kmod_list *modpost = NULL;
>    struct kmod_list *itr, *l;
>
>    ctx = kmod_new(NULL, NULL);
>
>    err = kmod_module_new_from_lookup(ctx, "cifs", &list);
>    if (err < 0) {
>        perror("kmod_module_new_from_lookup");
>        return EXIT_FAILURE;
>    }
>
>    kmod_list_foreach(l, list) {
>        struct kmod_module *mod = NULL;
>        mod = kmod_module_get_module(l);
>
>        kmod_module_get_softdeps(mod, &modpre, &modpost);
>
>        kmod_list_foreach(itr, modpre) {
>            struct kmod_module *mod = NULL;
>            const char *path = NULL;
>            mod = kmod_module_get_module(itr);
>            path = kmod_module_get_path(mod);
>            puts(path);
>        }
>    }
>}

  reply index

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-28 13:16 Harald Hoyer
2019-06-28 16:25 ` Lucas De Marchi [this message]
2019-06-29  9:30   ` Jean Delvare
2019-06-30  5:57     ` Steve French
2019-07-01 17:33       ` Paulo Alcantara

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190628162517.GA24484@ldmartin-desk1 \
    --to=lucas.de.marchi@gmail.com \
    --cc=harald@redhat.com \
    --cc=jdelvare@suse.de \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=pshilov@microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Modules Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-modules/0 linux-modules/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-modules linux-modules/ https://lore.kernel.org/linux-modules \
		linux-modules@vger.kernel.org linux-modules@archiver.kernel.org
	public-inbox-index linux-modules


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-modules


AGPL code for this site: git clone https://public-inbox.org/ public-inbox