linux-kernel - Re: [PATCH v2 2/3] lib/vsprintf: Split out sprintf() and friends

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:   Thu, 10 Aug 2023 10:15:17 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Marco Elver <elver@...gle.com>, linux-kernel@...r.kernel.org,
        kasan-dev@...glegroups.com, linux-mm@...ck.org,
        Steven Rostedt <rostedt@...dmis.org>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        Alexander Potapenko <glider@...gle.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v2 2/3] lib/vsprintf: Split out sprintf() and friends

On Tue 2023-08-08 15:47:59, Andy Shevchenko wrote:
> On Tue, Aug 08, 2023 at 08:41:49AM +0200, Petr Mladek wrote:
> > On Mon 2023-08-07 18:13:57, Andy Shevchenko wrote:
> > > On Mon, Aug 07, 2023 at 06:11:24PM +0300, Andy Shevchenko wrote:
> > > > On Mon, Aug 07, 2023 at 06:09:54PM +0300, Andy Shevchenko wrote:
> > > > > On Mon, Aug 07, 2023 at 05:03:19PM +0200, Petr Mladek wrote:
> > > > > > On Sat 2023-08-05 20:50:26, Andy Shevchenko wrote:
> 
> ...
> 
> > > > > > How does this sound, please?
> > > > > 
> > > > > Not every user (especially _header_) wants to have printk.h included just for
> > > > > sprintf.h that may have nothing to do with real output. So, same reasoning
> > > > > from me as keeping that in kernel.h, i.e. printk.h no better.
> > > > 
> > > > (haven't check these, just to show how many _headers_ uses sprintf() call)
> > > > 
> > > > $ git grep -lw s.*printf -- include/linux/
> > > > include/linux/acpi.h
> > > > include/linux/audit.h
> > > > include/linux/btf.h
> > > > include/linux/dev_printk.h
> > > > include/linux/device-mapper.h
> > > > include/linux/efi.h
> > > > include/linux/fortify-string.h
> > > > include/linux/fs.h
> > > > include/linux/gameport.h
> > > > include/linux/kdb.h
> > > > include/linux/kdev_t.h
> > > > include/linux/kernel.h
> > > > include/linux/mmiotrace.h
> > > > include/linux/netlink.h
> > > > include/linux/pci-p2pdma.h
> > > > include/linux/perf_event.h
> > > > include/linux/printk.h
> > > > include/linux/seq_buf.h
> > > > include/linux/seq_file.h
> > > > include/linux/shrinker.h
> > > > include/linux/string.h
> > > > include/linux/sunrpc/svc_xprt.h
> > > > include/linux/tnum.h
> > > > include/linux/trace_seq.h
> > > > include/linux/usb.h
> > > > include/linux/usb/gadget_configfs.h
> > > 
> > > Okay, revised as my regexp was too lazy
> > > 
> > > $ git grep -lw s[^[:space:]_]*printf -- include/linux/
> > > include/linux/btf.h
> > > include/linux/device-mapper.h
> > > include/linux/efi.h
> > > include/linux/fortify-string.h
> > > include/linux/kdev_t.h
> > > include/linux/kernel.h
> > > include/linux/netlink.h
> > > include/linux/pci-p2pdma.h
> > > include/linux/perf_event.h
> > > include/linux/sunrpc/svc_xprt.h
> > > include/linux/tnum.h
> > > include/linux/usb.h
> > > include/linux/usb/gadget_configfs.h
> > 
> > This is only a tiny part of the picture.
> > 
> > $> git grep sc*n*printf | cut -d : -f1 | uniq | grep "\.c$" | wc -l
> > 5254
> > $> find . -name  "*.c" | wc -l
> > 32319
> > 
> > It means that the vsprintf() family is used in 1/6 of all kernel
> > source files. They would need to include one extra header.
> 
> No, not only one. more, but the outcome of this is not using what is not used
> and unwinding the header dependency hell.
> 
> But hey, I am not talking about C files right now, it's secondary, however
> in IIO we want to get rid of kernel.h in the C files as well.

This sounds scary. Headers and C files are closely related. IMHO, it
does not makes sense to split header files without looking how
the functions are used.

Everyone agrees that kernel.h should be removed. But there are always
more possibilities where to move the definitions. For this, the use
in C files must be considered. Otherwise, it is just a try&hope approach.

> Also, please, go through all of them and tell, how many of them are using
> stuff from kernel.h besides sprintf.h and ARRAY_SIZE() (which I plan
> for a long time to split from kernel.h)?

I am all for removing vsprintf declarations from linux.h.

I provided the above numbers to support the idea of moving them
into printk.h.

The numbers show that the vsprintf function famility is used
quite frequently. IMHO, creating an extra tiny include file
will create more harm then good. By the harm I mean:

    + churn when updating 1/6 of source files

    + prolonging the list of #include lines in .c file. It will
      not help with maintainability which was one of the motivation
      in this patchset.

    + an extra work for people using vsprintf function family in
      new .c files. People are used to get them for free,
      together with printk().

Best Regards,
Petr