linux-kernel - Re: [PATCH] [PATCH] cxl: trace: Fix macro safety in CXL_EVT_TP_fast

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: <68096119523f6_1bed7d294ad@iweiny-mobl.notmuch>
Date: Wed, 23 Apr 2025 16:52:25 -0500
From: Ira Weiny <ira.weiny@...el.com>
To: shaikh.kamal <shaikhkamal2012@...il.com>, <dan.j.williams@...el.com>
CC: shaikh.kamal <shaikhkamal2012@...il.com>, Davidlohr Bueso
	<dave@...olabs.net>, Jonathan Cameron <jonathan.cameron@...wei.com>, "Dave
 Jiang" <dave.jiang@...el.com>, Alison Schofield <alison.schofield@...el.com>,
	Vishal Verma <vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>,
	Shiju Jose <shiju.jose@...wei.com>, "Fabio M. De Francesco"
	<fabio.m.de.francesco@...ux.intel.com>, "Steven Rostedt (Google)"
	<rostedt@...dmis.org>, Smita Koralahalli
	<Smita.KoralahalliChannabasappa@....com>, <linux-cxl@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] [PATCH] cxl: trace: Fix macro safety in
 CXL_EVT_TP_fast_assign

shaikh.kamal wrote:
> Fix checkpatch.pl detected error
> The CXL_EVT_TP_fast_assign macro assigns multiple fields, but does not
> wrap the body in a `do { ... } while (0)` block. This can lead to
> unexpected behavior when used in conditional branches.
> 
> Add checks to ensure cxlmd is valid before accessing its fields.
> 
> Signed-off-by: shaikh.kamal <shaikhkamal2012@...il.com>

Nak

This is not a normal macro.  checkpatch.pl does not apply in this case.
Also checking cxlmd here is not necessary.  The callers of the tracepoint
know very well that it needs to be valid.

Ira

> ---
>  drivers/cxl/core/trace.h | 30 ++++++++++++++++++------------
>  1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
> index 25ebfbc1616c..a1a1014ee1fe 100644
> --- a/drivers/cxl/core/trace.h
> +++ b/drivers/cxl/core/trace.h
> @@ -249,18 +249,24 @@ TRACE_EVENT(cxl_overflow,
>  	__field(u8, hdr_maint_op_class)				\
>  	__field(u8, hdr_maint_op_sub_class)
>  
> -#define CXL_EVT_TP_fast_assign(cxlmd, l, hdr)					\
> -	__assign_str(memdev);				\
> -	__assign_str(host);			\
> -	__entry->log = (l);							\
> -	__entry->serial = (cxlmd)->cxlds->serial;				\
> -	__entry->hdr_length = (hdr).length;					\
> -	__entry->hdr_flags = get_unaligned_le24((hdr).flags);			\
> -	__entry->hdr_handle = le16_to_cpu((hdr).handle);			\
> -	__entry->hdr_related_handle = le16_to_cpu((hdr).related_handle);	\
> -	__entry->hdr_timestamp = le64_to_cpu((hdr).timestamp);			\
> -	__entry->hdr_maint_op_class = (hdr).maint_op_class;			\
> -	__entry->hdr_maint_op_sub_class = (hdr).maint_op_sub_class
> +#define CXL_EVT_TP_fast_assign(cxlmd, l, hdr) \
> +	do { \
> +		if (!(cxlmd)) { \
> +			pr_err("Invalid arguments to CXL_EVT_TP_fast_assign\n"); \
> +			break; \
> +		} \
> +		__assign_str(memdev); \
> +		__assign_str(host); \
> +		__entry->log = (l); \
> +		__entry->serial = (cxlmd)->cxlds->serial; \
> +		__entry->hdr_length = (hdr).length; \
> +		__entry->hdr_flags = get_unaligned_le24((hdr).flags); \
> +		__entry->hdr_handle = le16_to_cpu((hdr).handle); \
> +		__entry->hdr_related_handle = le16_to_cpu((hdr).related_handle); \
> +		__entry->hdr_timestamp = le64_to_cpu((hdr).timestamp); \
> +		__entry->hdr_maint_op_class = (hdr).maint_op_class; \
> +		__entry->hdr_maint_op_sub_class = (hdr).maint_op_sub_class; \
> +	} while (0)
>  
>  #define CXL_EVT_TP_printk(fmt, ...) \
>  	TP_printk("memdev=%s host=%s serial=%lld log=%s : time=%llu uuid=%pUb "	\
> -- 
> 2.43.0
>