| 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
| ||
|
Message-ID: <5661BFEE.1050103@akamai.com>
Date: Fri, 4 Dec 2015 11:31:42 -0500
From: Jason Baron <jbaron@...mai.com>
To: Aaron Conole <aconole@...hat.com>,
Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-kernel@...r.kernel.org, Joe Perches <joe@...ches.com>
Subject: Re: [PATCH] printk: fix pr_debug and pr_devel to elide function calls
On 12/03/2015 05:45 PM, Aaron Conole wrote:
> Currently, pr_debug and pr_devel will not elide function call arguments
> appearing in calls to no_printk for these macros. This is because all
> side effects must be honored before proceeding to the 0-value assignment
> in no_printk.
>
> The behavior is contrary to documentation found in the CodingStyle and
> header file where these functions are declared.
>
> This patch corrects that behavior by shunting out the call to no_printk
> completely. The format string is still checked by gcc for correctness, but
> no code seems to be emitted in common cases.
>
> fixes commit 5264f2f75d86 ("include/linux/printk.h: use and neaten
> no_printk")
>
> Signed-off-by: Aaron Conole <aconole@...hat.com>
> Reported-by: Dmitry Vyukov <dvyukov@...gle.com>
> Cc: Joe Perches <joe@...ches.com>
I think we should just convert no_printk() to not emit anything. This
will avoid us adding unwrapped calls to 'no_printk()' in the future, and
I think makes the code more readable. Based on Joe's previous
'eliminated_printk()' thing. IE:
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 9729565..58632bf 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -108,11 +108,11 @@ struct va_format {
* Dummy printk for disabled debugging statements to use whilst maintaining
* gcc's format and side-effect checking.
*/
-static inline __printf(1, 2)
-int no_printk(const char *fmt, ...)
-{
- return 0;
-}
+#define no_printk(fmt, ...) \
+do { \
+ if (0) \
+ printk(fmt, ##__VA_ARGS__); \
+} while (0)
#ifdef CONFIG_EARLY_PRINTK
extern asmlinkage __printf(1, 2)
Thanks,
-Jason
>
> ---
> include/linux/printk.h | 42 +++++++++++++++++++++++++++++++++---------
> 1 file changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index 9729565..87dfdd2 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -270,8 +270,12 @@ extern asmlinkage void dump_stack(void) __cold;
> #define pr_devel(fmt, ...) \
> printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> #else
> -#define pr_devel(fmt, ...) \
> - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_devel(fmt, ...) \
> +do { \
> + if (0) { \
> + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\
> + } \
> +} while (0)
> #endif
>
> #include <linux/dynamic_debug.h>
> @@ -286,7 +290,11 @@ extern asmlinkage void dump_stack(void) __cold;
> printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> #else
> #define pr_debug(fmt, ...) \
> - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +do { \
> + if (0) { \
> + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\
> + } \
> +} while (0)
> #endif
>
> /*
> @@ -341,7 +349,11 @@ extern asmlinkage void dump_stack(void) __cold;
> printk_once(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> #else
> #define pr_devel_once(fmt, ...) \
> - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +do { \
> + if (0) { \
> + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\
> + } \
> +} while (0)
> #endif
>
> /* If you are writing a driver, please use dev_dbg instead */
> @@ -350,7 +362,11 @@ extern asmlinkage void dump_stack(void) __cold;
> printk_once(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> #else
> #define pr_debug_once(fmt, ...) \
> - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +do { \
> + if (0) { \
> + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\
> + } \
> +} while (0)
> #endif
>
> /*
> @@ -392,8 +408,12 @@ extern asmlinkage void dump_stack(void) __cold;
> #define pr_devel_ratelimited(fmt, ...) \
> printk_ratelimited(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> #else
> -#define pr_devel_ratelimited(fmt, ...) \
> - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_devel_ratelimited(fmt, ...) \
> +do { \
> + if (0) { \
> + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\
> + } \
> +} while (0)
> #endif
>
> /* If you are writing a driver, please use dev_dbg instead */
> @@ -413,8 +433,12 @@ do { \
> #define pr_debug_ratelimited(fmt, ...) \
> printk_ratelimited(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> #else
> -#define pr_debug_ratelimited(fmt, ...) \
> - no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> +#define pr_debug_ratelimited(fmt, ...) \
> +do { \
> + if (0) { \
> + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);\
> + } \
> +} while (0)
> #endif
>
> extern const struct file_operations kmsg_fops;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists