linux-kernel - Re: [PATCH BUGFIX] block: make elevator_get robust against cross blk/blk-mq choice

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1487020366.2719.5.camel@sandisk.com>
Date:   Mon, 13 Feb 2017 21:12:59 +0000
From:   Bart Van Assche <Bart.VanAssche@...disk.com>
To:     "tj@...nel.org" <tj@...nel.org>,
        "paolo.valente@...aro.org" <paolo.valente@...aro.org>,
        "axboe@...nel.dk" <axboe@...nel.dk>
CC:     "ulf.hansson@...aro.org" <ulf.hansson@...aro.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "linus.walleij@...aro.org" <linus.walleij@...aro.org>
Subject: Re: [PATCH BUGFIX] block: make elevator_get robust against cross
 blk/blk-mq choice

On Mon, 2017-02-13 at 22:01 +0100, Paolo Valente wrote:
> -static struct elevator_type *elevator_get(const char *name, bool try_loading)
> +static struct elevator_type *elevator_get(const char *name, bool try_loading,
> +					  bool mq_ops)

Please choose a better name for that argument, e.q. "mq". To me the name "mq_ops"
means "a pointer to a data structure with operation function pointers".

> +	if (e && (e->uses_mq != mq_ops)) {
> +		pr_err("ERROR: attempted to choose %s %s I/O scheduler in blk%s",
> +		       name, e->uses_mq ? "blk-mq" : "legacy", mq_ops ? "-mq" : "");
> +		e = NULL;
> +	}

How about changing the above into:

+       if (e && e->uses_mq != mq) {
+               pr_err("ERROR: attempt to configure %s as I/O scheduler for a %s queue\n",
+                      name, mq ? "blk-mq" : "legacy");
+               e = NULL;
+       }

Thanks,

Bart.