netdev - Re: [net-next 01/13] iavf: Use printf instead of gnu_printf for iavf_debug

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, 31 May 2019 09:08:23 -0700
From:   Joe Perches <joe@...ches.com>
To:     Jeff Kirsher <jeffrey.t.kirsher@...el.com>, davem@...emloft.net
Cc:     Nathan Chancellor <natechancellor@...il.com>,
        netdev@...r.kernel.org, nhorman@...hat.com, sassmann@...hat.com,
        Miguel Ojeda <miguel.ojeda.sandonis@...il.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Andrew Bowers <andrewx.bowers@...el.com>
Subject: Re: [net-next 01/13] iavf: Use printf instead of gnu_printf for
 iavf_debug_d

On Fri, 2019-05-31 at 01:15 -0700, Jeff Kirsher wrote:
> From: Nathan Chancellor <natechancellor@...il.com>
> We can convert from gnu_printf to printf without any side effects
[]
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_osdep.h b/drivers/net/ethernet/intel/iavf/iavf_osdep.h
[]
> @@ -46,7 +46,7 @@ struct iavf_virt_mem {
>  
>  #define iavf_debug(h, m, s, ...)  iavf_debug_d(h, m, s, ##__VA_ARGS__)
>  extern void iavf_debug_d(void *hw, u32 mask, char *fmt_str, ...)
> -	__attribute__ ((format(gnu_printf, 3, 4)));
> +	__printf(3, 4);

This declaration and function likely have several defects:

void *hw should likely be struct iavf_hw *hw
char *fmt_str should likely be const char *

And the definition of the function should
probably have the output at KERN_DEBUG and not
KERN_INFO.  As well it should probably use %pV
instead of a local automatic buffer.

Perhaps a macro like:

#define iavf_debug_d(hw, mask, fmt, ...)	\
do {						\
	if ((mask) & (hw)->debug_mask)		\
		pr_debug(fmt, ##__VA_ARGS__);	\
} while (0)

would be better as it could allow dynamic debug
for every use.