netdev - Re: [PATCH] atm/clip: Use seq_puts() in svc

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] [day] [month] [year] [list]

Date:   Mon, 8 Jan 2018 08:25:16 +0100
From:   SF Markus Elfring <elfring@...rs.sourceforge.net>
To:     Andy Shevchenko <andy.shevchenko@...il.com>,
        netdev <netdev@...r.kernel.org>
Cc:     Bhumika Goyal <bhumirks@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        David Windsor <dwindsor@...il.com>,
        Elena Reshetova <elena.reshetova@...el.com>,
        Hans Liljestrand <ishkamiel@...il.com>,
        Johannes Berg <johannes.berg@...el.com>,
        Kees Cook <keescook@...omium.org>,
        Roopa Prabhu <roopa@...ulusnetworks.com>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] atm/clip: Use seq_puts() in svc_addr()

>> @@ -708,11 +708,11 @@ static void svc_addr(struct seq_file *seq, struct sockaddr_atmsvc *addr)
>>         static int e164[] = { 1, 8, 4, 6, 1, 0 };
>>
>>         if (*addr->sas_addr.pub) {
>> -               seq_printf(seq, "%s", addr->sas_addr.pub);
>> +               seq_puts(seq, addr->sas_addr.pub);
> 
> Which opens a lot of security concerns.

How? - The passed string is just copied into a buffer finally, isn't it?

> Never do this again.

Why do you not like such a small source code transformation at the moment?

> P.S. I'm wondering what would be first,

I am curious on how communication difficulties can be adjusted.

> Markus starts looking into the actual code,

I inspected the original source code to some degree.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/fs/seq_file.c?id=895c0dde398510a5b5ded60e5064c11b94bd30ca#n682
https://elixir.free-electrons.com/linux/v4.15-rc6/source/fs/seq_file.c#L660

> or most (all) of the maintainers just ban him.

The change acceptance is varying for various reasons by the involved contributors.

Regards,
Markus