netdev - Re: [PATCH 08/12] cdrom: use correct format characters

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:   Mon, 13 Jun 2022 11:47:18 -0700
From:   Bill Wendling <morbo@...gle.com>
To:     Phillip Potter <phil@...lpotter.co.uk>
Cc:     Bill Wendling <isanbard@...il.com>,
        Tony Luck <tony.luck@...el.com>,
        Borislav Petkov <bp@...en8.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>, Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        Jan Kara <jack@...e.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Pablo Neira Ayuso <pablo@...filter.org>,
        Jozsef Kadlecsik <kadlec@...filter.org>,
        Florian Westphal <fw@...len.de>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Tom Rix <trix@...hat.com>,
        Ross Philipson <ross.philipson@...cle.com>,
        Daniel Kiper <daniel.kiper@...cle.com>,
        linux-edac@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
        ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
        linux-mm@...ck.org, netfilter-devel@...r.kernel.org,
        coreteam@...filter.org, Networking <netdev@...r.kernel.org>,
        alsa-devel@...a-project.org,
        clang-built-linux <llvm@...ts.linux.dev>
Subject: Re: [PATCH 08/12] cdrom: use correct format characters

On Sun, Jun 12, 2022 at 9:23 AM Phillip Potter <phil@...lpotter.co.uk> wrote:
>
> On Thu, Jun 09, 2022 at 10:16:27PM +0000, Bill Wendling wrote:
> > From: Bill Wendling <isanbard@...il.com>
> >
> > When compiling with -Wformat, clang emits the following warnings:
> >
> > drivers/cdrom/cdrom.c:3454:48: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
> >         ret = scnprintf(info + *pos, max_size - *pos, header);
> >                                                       ^~~~~~
> >
> > Use a string literal for the format string.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/378
> > Signed-off-by: Bill Wendling <isanbard@...il.com>
> > ---
> >  drivers/cdrom/cdrom.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> > index 416f723a2dbb..52b40120c76e 100644
> > --- a/drivers/cdrom/cdrom.c
> > +++ b/drivers/cdrom/cdrom.c
> > @@ -3451,7 +3451,7 @@ static int cdrom_print_info(const char *header, int val, char *info,
> >       struct cdrom_device_info *cdi;
> >       int ret;
> >
> > -     ret = scnprintf(info + *pos, max_size - *pos, header);
> > +     ret = scnprintf(info + *pos, max_size - *pos, "%s", header);
> >       if (!ret)
> >               return 1;
> >
> > --
> > 2.36.1.255.ge46751e96f-goog
> >
>
> Hi Bill,
>
> Thank you for the patch, much appreciated.
>
> Looking at this though, all callers of cdrom_print_info() provide 'header'
> as a string literal defined within the driver, when making the call.
> Therefore, I'm not convinced this change is necessary for cdrom.c -
> that said, in this particular use case I don't think it would hurt
> either.
>
> I've followed the other responses on parts of this series, so I
> understand that a different solution is potentially in the works.
> Thought I'd respond anyway though out of courtesy.
>
Thanks, Phillip.

I pointed out in a separate response that this specific warning is
disabled by default, but when I ran into while hacking stuff there
weren't a lot of places where the warning popped up (at least for x86
builds) and thought it would be a nice cleanup. I understand if you
don't think this patch is necessary for your code. There are some
places where visual inspection of the code is "good enough" to ensure
that nothing untoward will happen (Greg pointed out a similar thing in
an mm/ file).

Cheers!
-bw