linux-hardening - Re: [PATCH -next v3 1/3] lib/string_choices: Add str_true_false()/str_false

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 for Android: free password hash cracker in your pocket

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

Message-ID: <3f40ff3c-0f66-49cc-806f-1cab6c8a8c50@huawei.com>
Date: Thu, 29 Aug 2024 09:12:45 +0800
From: Hongbo Li <lihongbo22@...wei.com>
To: Andrew Morton <akpm@...ux-foundation.org>
CC: <kees@...nel.org>, <andy@...nel.org>, <trondmy@...nel.org>,
	<anna@...nel.org>, <gregkh@...uxfoundation.org>,
	<linux-hardening@...r.kernel.org>, <linux-mm@...ck.org>,
	<linux-nfs@...r.kernel.org>
Subject: Re: [PATCH -next v3 1/3] lib/string_choices: Add
 str_true_false()/str_false_true() helper



On 2024/8/29 4:25, Andrew Morton wrote:
> On Wed, 28 Aug 2024 11:49:51 +0800 Hongbo Li <lihongbo22@...wei.com> wrote:
> 
>>> Anything which is calling these functions is not performance-sensitive,
>>> so optimizing for space is preferred.  An out-of-line function which
>>> returns a const char * will achieve this?
>> I think this helper can achieve this. Because it is tiny enough, the
>> compiler will handle this like #define macro (do the replacement)
>> without allocating extra functional stack. On the contrary, if it is
>> implemented as a non-inline function, it will cause extra functional
>> stack when it was called every time. And it also should be implemented
>> in a source file (.c file), not in header file(.h file).
> 
> No, my concern is that if, for example, str_high_low() gets used in 100
> .c files then we get 100 copies of the strings "high" and "low" in
> vmlinux.  Making str_high_low() uninlined would fix this.

At first, I didn't consider these aspects.

> 
> However a quick experiment tells me that the compiler and linker are
> indeed able to perform this cross-object-file optimization:
> 
That's very good!

Thanks,
Hongbo

> --- a/fs/open.c~a
> +++ a/fs/open.c
> @@ -34,6 +34,8 @@
>   #include <linux/mnt_idmapping.h>
>   #include <linux/filelock.h>
>   
> +#include <linux/string_choices.h>
> +
>   #include "internal.h"
>   
>   int do_truncate(struct mnt_idmap *idmap, struct dentry *dentry,
> @@ -42,6 +44,8 @@ int do_truncate(struct mnt_idmap *idmap,
>   	int ret;
>   	struct iattr newattrs;
>   
> +	printk("%s\n", frozzle(dentry == NULL));
> +
>   	/* Not pretty: "inode->i_size" shouldn't really be signed. But it is. */
>   	if (length < 0)
>   		return -EINVAL;
> --- a/fs/inode.c~a
> +++ a/fs/inode.c
> @@ -22,6 +22,9 @@
>   #include <linux/iversion.h>
>   #include <linux/rw_hint.h>
>   #include <trace/events/writeback.h>
> +
> +#include <linux/string_helpers.h>
> +
>   #include "internal.h"
>   
>   /*
> @@ -110,6 +113,8 @@ static struct inodes_stat_t inodes_stat;
>   static int proc_nr_inodes(const struct ctl_table *table, int write, void *buffer,
>   			  size_t *lenp, loff_t *ppos)
>   {
> +	printk("%s\n", frozzle(table == NULL));
> +
>   	inodes_stat.nr_inodes = get_nr_inodes();
>   	inodes_stat.nr_unused = get_nr_inodes_unused();
>   	return proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
> --- a/include/linux/string_choices.h~a
> +++ a/include/linux/string_choices.h
> @@ -4,6 +4,11 @@
>   
>   #include <linux/types.h>
>   
> +static inline const char *frozzle(bool v)
> +{
> +	return v ? "frizzle" : "frazzle";
> +}
> +
>   static inline const char *str_enable_disable(bool v)
>   {
>   	return v ? "enable" : "disable";
> _
> 
> 
> x1:/usr/src/25> strings vmlinux|grep frazzle
> frazzle
> x1:/usr/src/25>
> 
> See, only one copy!
>