linux-kernel - Re: [RFC PATCH 12/13] blk-mq.h: Fix parentheses around macro parameter use

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]

Date:   Fri, 5 May 2023 09:56:41 -0400
From:   Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To:     Andrew Morton <akpm@...ux-foundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     linux-kernel@...r.kernel.org, Jens Axboe <axboe@...nel.dk>,
        linux-block@...r.kernel.org
Subject: Re: [RFC PATCH 12/13] blk-mq.h: Fix parentheses around macro
 parameter use

On 2023-05-04 16:05, Mathieu Desnoyers wrote:
> Fix the following macro parameter usage patterns in blk-mq.h for
> consistency, ensuring that operator precedence is respected:
> 
> Added parentheses:
[...]
> - "x = y" is changed for "x = (y)", because "y" can be an expression
>    containing a comma if it is the result of the expansion of a macro such
>    as #define eval(...) __VA_ARGS__, which would cause unexpected operator
>    precedence. This use-case is far-fetched, but we have to choose one
>    way or the other (with or without parentheses) for consistency.

[...]

>   include/linux/blk-mq.h | 38 +++++++++++++++++++-------------------
>   1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index 06caacd77ed6..4de6ad92530c 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -223,13 +223,13 @@ static inline unsigned short req_get_ioprio(struct request *req)
>   
>   #define rq_list_add(listptr, rq)	do {		\
>   	(rq)->rq_next = *(listptr);			\
> -	*(listptr) = rq;				\
> +	*(listptr) = (rq);				\
>   } while (0)
> 

Linus,

Which way do we want to go with respect to the rvalue of the assignment 
operator "=" in a macro ? (with or without parentheses)

In short:

#define m(x) do { z = (x); } while (0)

or

#define m(x) do { z = x; } while (0)

?

Given that "=" has the lowest operator precedence just above comma, and 
its associativity is right-to-left, I suspect the only use that would 
break it without the extra parentheses around "x" is:

#define eval(...) __VA_ARGS__
#define m(x) do { z = x; } while (0)

m(eval(1, abc))

Which generates the following C code after preprocessing:

do { z = 1, abc; } while (0)

which ends up expanding the comma within the rvalue. But this use-case 
is a bit far-fetched, so I don't know if we want to require the 
parentheses or not.

And if we decide that we do want to require the parentheses around the 
"x" parameter in the "=" rvalue, then this means we have to consider 
whether we want to require parentheses around the macro arguments used 
as function/macro arguments, e.g.:

#define eval(...) __VA_ARGS__
#define m(x)    f(x)

m(eval(1, abc));

Which generates the following C code after preprocessing:

f(1, abc);

If we want to be consistent, I suspect we want to require the same for 
both use-cases ("=" rvalue and function/macro parameters).

Thanks,

Mathieu



-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com