[{"id":1766263,"web_url":"http://patchwork.ozlabs.org/comment/1766263/","msgid":"<20170911115511.GB16198@kroah.com>","list_archive_url":null,"date":"2017-09-11T11:55:11","subject":"Re: [PATCH 2/3] PCI: don't use snprintf() in driver_override_show()","submitter":{"id":11800,"url":"http://patchwork.ozlabs.org/api/people/11800/","name":"Greg Kroah-Hartman","email":"gregkh@linuxfoundation.org"},"content":"On Mon, Sep 11, 2017 at 09:45:41AM +0200, Nicolai Stange wrote:\n> Quote from Documentation/filesystems/sysfs.txt:\n> \n>   show() must not use snprintf() when formatting the value to be\n>   returned to user space. If you can guarantee that an overflow\n>   will never happen you can use sprintf() otherwise you must use\n>   scnprintf().\n> \n> Commit 4efe874aace5 (\"PCI: Don't read past the end of sysfs\n> \"driver_override\" buffer\") introduced such a snprintf() usage from\n> driver_override_show() while at the same time tweaking\n> driver_override_store() such that the write buffer can't ever get\n> overflowed.\n> \n> Reasoning:\n> Since aforementioned commit, driver_override_store() only accepts to be\n> written buffers less than PAGE_SIZE - 1 in size.\n> \n> The then kstrndup()'ed driver_override string will be at most PAGE_SIZE - 1\n> in length, including the trailing '\\0'.\n> \n> After the addition of a '\\n' in driver_override_show(), the result won't\n> exceed PAGE_SIZE characters in length, again including the trailing '\\0'.\n> \n> Hence, snprintf(buf, PAGE_SIZE, ...) and sprintf(buf, ...) are equivalent\n> at this point.\n> \n> Replace the former by the latter in order to adhere to the rules in\n> Documentation/filesystems/sysfs.txt.\n> \n> This is a style fix only and there's no change in functionality.\n> \n> Signed-off-by: Nicolai Stange <nstange@suse.de>\n> ---\n>  drivers/pci/pci-sysfs.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c\n> index 8e075ea2743e..43f7fbede448 100644\n> --- a/drivers/pci/pci-sysfs.c\n> +++ b/drivers/pci/pci-sysfs.c\n> @@ -722,7 +722,7 @@ static ssize_t driver_override_show(struct device *dev,\n>  \tssize_t len;\n>  \n>  \tdevice_lock(dev);\n> -\tlen = snprintf(buf, PAGE_SIZE, \"%s\\n\", pdev->driver_override);\n> +\tlen = sprintf(buf, \"%s\\n\", pdev->driver_override);\n\nWhile I'm all for changes like this, it's an uphill battle to change\nthem, usually it's best to just catch them before they go into the tree.\n\nAnyway, nice summary, very good job with that.\n\nAcked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>","headers":{"Return-Path":"<linux-pci-owner@vger.kernel.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-pci-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xrRvm1kDHz9s71\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon, 11 Sep 2017 22:25:08 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751191AbdIKLzN (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tMon, 11 Sep 2017 07:55:13 -0400","from mail.linuxfoundation.org ([140.211.169.12]:41464 \"EHLO\n\tmail.linuxfoundation.org\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750989AbdIKLzN (ORCPT\n\t<rfc822; linux-pci@vger.kernel.org>); Mon, 11 Sep 2017 07:55:13 -0400","from localhost (h30.122.139.40.ip.windstream.net [40.139.122.30])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPSA id 9DB6BA7A;\n\tMon, 11 Sep 2017 11:55:12 +0000 (UTC)"],"Date":"Mon, 11 Sep 2017 04:55:11 -0700","From":"Greg Kroah-Hartman <gregkh@linuxfoundation.org>","To":"Nicolai Stange <nstange@suse.de>","Cc":"Bjorn Helgaas <bhelgaas@google.com>, Adrian Salido <salidoa@google.com>,\n\tSasha Levin <sasha.levin@oracle.com>,\n\tlinux-kernel@vger.kernel.org, linux-pci@vger.kernel.org","Subject":"Re: [PATCH 2/3] PCI: don't use snprintf() in driver_override_show()","Message-ID":"<20170911115511.GB16198@kroah.com>","References":"<20170911074542.16777-1-nstange@suse.de>\n\t<20170911074542.16777-3-nstange@suse.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170911074542.16777-3-nstange@suse.de>","User-Agent":"Mutt/1.9.0 (2017-09-02)","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<linux-pci.vger.kernel.org>","X-Mailing-List":"linux-pci@vger.kernel.org"}},{"id":1775047,"web_url":"http://patchwork.ozlabs.org/comment/1775047/","msgid":"<20170925234819.GH15970@bhelgaas-glaptop.roam.corp.google.com>","list_archive_url":null,"date":"2017-09-25T23:48:19","subject":"Re: [PATCH 2/3] PCI: don't use snprintf() in driver_override_show()","submitter":{"id":67298,"url":"http://patchwork.ozlabs.org/api/people/67298/","name":"Bjorn Helgaas","email":"helgaas@kernel.org"},"content":"On Mon, Sep 11, 2017 at 04:55:11AM -0700, Greg Kroah-Hartman wrote:\n> On Mon, Sep 11, 2017 at 09:45:41AM +0200, Nicolai Stange wrote:\n> > Quote from Documentation/filesystems/sysfs.txt:\n> > \n> >   show() must not use snprintf() when formatting the value to be\n> >   returned to user space. If you can guarantee that an overflow\n> >   will never happen you can use sprintf() otherwise you must use\n> >   scnprintf().\n> > \n> > Commit 4efe874aace5 (\"PCI: Don't read past the end of sysfs\n> > \"driver_override\" buffer\") introduced such a snprintf() usage from\n> > driver_override_show() while at the same time tweaking\n> > driver_override_store() such that the write buffer can't ever get\n> > overflowed.\n> > \n> > Reasoning:\n> > Since aforementioned commit, driver_override_store() only accepts to be\n> > written buffers less than PAGE_SIZE - 1 in size.\n> > \n> > The then kstrndup()'ed driver_override string will be at most PAGE_SIZE - 1\n> > in length, including the trailing '\\0'.\n> > \n> > After the addition of a '\\n' in driver_override_show(), the result won't\n> > exceed PAGE_SIZE characters in length, again including the trailing '\\0'.\n> > \n> > Hence, snprintf(buf, PAGE_SIZE, ...) and sprintf(buf, ...) are equivalent\n> > at this point.\n> > \n> > Replace the former by the latter in order to adhere to the rules in\n> > Documentation/filesystems/sysfs.txt.\n> > \n> > This is a style fix only and there's no change in functionality.\n> > \n> > Signed-off-by: Nicolai Stange <nstange@suse.de>\n> > ---\n> >  drivers/pci/pci-sysfs.c | 2 +-\n> >  1 file changed, 1 insertion(+), 1 deletion(-)\n> > \n> > diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c\n> > index 8e075ea2743e..43f7fbede448 100644\n> > --- a/drivers/pci/pci-sysfs.c\n> > +++ b/drivers/pci/pci-sysfs.c\n> > @@ -722,7 +722,7 @@ static ssize_t driver_override_show(struct device *dev,\n> >  \tssize_t len;\n> >  \n> >  \tdevice_lock(dev);\n> > -\tlen = snprintf(buf, PAGE_SIZE, \"%s\\n\", pdev->driver_override);\n> > +\tlen = sprintf(buf, \"%s\\n\", pdev->driver_override);\n> \n> While I'm all for changes like this, it's an uphill battle to change\n> them, usually it's best to just catch them before they go into the tree.\n> \n> Anyway, nice summary, very good job with that.\n> \n> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>\n\nWhy use snprintf() instead of scnprintf()?  It looks like snprintf()\nis probably safe, but requires a bunch of analysis to prove that,\nwhile scnprintf() would be obviously safe.\n\nBjorn","headers":{"Return-Path":"<linux-pci-owner@vger.kernel.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-pci-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","mail.kernel.org;\n\tdmarc=none (p=none dis=none) header.from=kernel.org","mail.kernel.org;\n\tspf=none smtp.mailfrom=helgaas@kernel.org"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y1LPg5X8Rz9t2Q\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 26 Sep 2017 09:48:23 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S966319AbdIYXsW (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tMon, 25 Sep 2017 19:48:22 -0400","from mail.kernel.org ([198.145.29.99]:55122 \"EHLO mail.kernel.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S966299AbdIYXsV (ORCPT <rfc822;linux-pci@vger.kernel.org>);\n\tMon, 25 Sep 2017 19:48:21 -0400","from localhost (unknown [69.71.4.159])\n\t(using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits))\n\t(No client certificate requested)\n\tby mail.kernel.org (Postfix) with ESMTPSA id 794162148C;\n\tMon, 25 Sep 2017 23:48:20 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mail.kernel.org 794162148C","Date":"Mon, 25 Sep 2017 18:48:19 -0500","From":"Bjorn Helgaas <helgaas@kernel.org>","To":"Greg Kroah-Hartman <gregkh@linuxfoundation.org>","Cc":"Nicolai Stange <nstange@suse.de>, Bjorn Helgaas <bhelgaas@google.com>,\n\tAdrian Salido <salidoa@google.com>, Sasha Levin <sasha.levin@oracle.com>,\n\tlinux-kernel@vger.kernel.org, linux-pci@vger.kernel.org","Subject":"Re: [PATCH 2/3] PCI: don't use snprintf() in driver_override_show()","Message-ID":"<20170925234819.GH15970@bhelgaas-glaptop.roam.corp.google.com>","References":"<20170911074542.16777-1-nstange@suse.de>\n\t<20170911074542.16777-3-nstange@suse.de>\n\t<20170911115511.GB16198@kroah.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170911115511.GB16198@kroah.com>","User-Agent":"Mutt/1.5.21 (2010-09-15)","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<linux-pci.vger.kernel.org>","X-Mailing-List":"linux-pci@vger.kernel.org"}},{"id":1775187,"web_url":"http://patchwork.ozlabs.org/comment/1775187/","msgid":"<87zi9ivuec.fsf@suse.de>","list_archive_url":null,"date":"2017-09-26T06:36:11","subject":"Re: [PATCH 2/3] PCI: don't use snprintf() in driver_override_show()","submitter":{"id":72345,"url":"http://patchwork.ozlabs.org/api/people/72345/","name":"Nicolai Stange","email":"nstange@suse.de"},"content":"Bjorn Helgaas <helgaas@kernel.org> writes:\n\n> On Mon, Sep 11, 2017 at 04:55:11AM -0700, Greg Kroah-Hartman wrote:\n>> On Mon, Sep 11, 2017 at 09:45:41AM +0200, Nicolai Stange wrote:\n>> > Quote from Documentation/filesystems/sysfs.txt:\n>> > \n>> >   show() must not use snprintf() when formatting the value to be\n>> >   returned to user space. If you can guarantee that an overflow\n>> >   will never happen you can use sprintf() otherwise you must use\n>> >   scnprintf().\n>> > \n>> > Commit 4efe874aace5 (\"PCI: Don't read past the end of sysfs\n>> > \"driver_override\" buffer\") introduced such a snprintf() usage from\n>> > driver_override_show() while at the same time tweaking\n>> > driver_override_store() such that the write buffer can't ever get\n>> > overflowed.\n>> > \n>> > Reasoning:\n>> > Since aforementioned commit, driver_override_store() only accepts to be\n>> > written buffers less than PAGE_SIZE - 1 in size.\n>> > \n>> > The then kstrndup()'ed driver_override string will be at most PAGE_SIZE - 1\n>> > in length, including the trailing '\\0'.\n>> > \n>> > After the addition of a '\\n' in driver_override_show(), the result won't\n>> > exceed PAGE_SIZE characters in length, again including the trailing '\\0'.\n>> > \n>> > Hence, snprintf(buf, PAGE_SIZE, ...) and sprintf(buf, ...) are equivalent\n>> > at this point.\n>> > \n>> > Replace the former by the latter in order to adhere to the rules in\n>> > Documentation/filesystems/sysfs.txt.\n>> > \n>> > This is a style fix only and there's no change in functionality.\n>> > \n>> > Signed-off-by: Nicolai Stange <nstange@suse.de>\n>> > ---\n>> >  drivers/pci/pci-sysfs.c | 2 +-\n>> >  1 file changed, 1 insertion(+), 1 deletion(-)\n>> > \n>> > diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c\n>> > index 8e075ea2743e..43f7fbede448 100644\n>> > --- a/drivers/pci/pci-sysfs.c\n>> > +++ b/drivers/pci/pci-sysfs.c\n>> > @@ -722,7 +722,7 @@ static ssize_t driver_override_show(struct device *dev,\n>> >  \tssize_t len;\n>> >  \n>> >  \tdevice_lock(dev);\n>> > -\tlen = snprintf(buf, PAGE_SIZE, \"%s\\n\", pdev->driver_override);\n>> > +\tlen = sprintf(buf, \"%s\\n\", pdev->driver_override);\n>> \n>> While I'm all for changes like this, it's an uphill battle to change\n>> them, usually it's best to just catch them before they go into the tree.\n\nI did this patch mostly for demonstrating why the exclusion of the\nsprintf() -> snprintf() change from the port of\n\n  3d713e0e382e (\"driver core: platform: add device binding path 'driver_override'\")\n\nto platform in [3/3] is safe. Because I'm pretty sure that [3/3] would\nhave been rejected if it had included that chunk -- due to the\nnon-conformity to Documentation/.\n\n\n>> Anyway, nice summary, very good job with that.\n>> \n>> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>\n>\n> Why use snprintf() instead of scnprintf()?  \n\ns/snprintf/sprintf/, right?\n\n\n> It looks like snprintf() is probably safe, but requires a bunch of\n> analysis to prove that, while scnprintf() would be obviously safe.\n\nPersonal preference, I guess. Something like \"using sprintf() would\nimplicitly document that the buffer is always large enough\", i.e. that\nthe size check in driver_override_store() is already strict enough / correct.\n\nAnyway, I don't have a strong opinion on that. So if you want me to\nchange this, I'll do, of course. Just note that Greg K-H. has queued up\n[3/3] somewhere already and thus, it would probably require two patches\nrather than only this one to make PCI and platform look the same again,\nprovided that's what is wanted.\n\nSo, given that this patch has fulfilled its purpose already, I'm fine\nwith either of\n- dropping it\n- taking it\n- s/sprintf/scnprintf/ and do the same for platform.\n\n\nThanks,\n\nNicolai","headers":{"Return-Path":"<linux-pci-owner@vger.kernel.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-pci-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y1WSY2V85z9sPr\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 26 Sep 2017 16:36:29 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1754241AbdIZGgQ (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tTue, 26 Sep 2017 02:36:16 -0400","from mx2.suse.de ([195.135.220.15]:33241 \"EHLO mx1.suse.de\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1752265AbdIZGgP (ORCPT <rfc822;linux-pci@vger.kernel.org>);\n\tTue, 26 Sep 2017 02:36:15 -0400","from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id 6E3FAAED0;\n\tTue, 26 Sep 2017 06:36:13 +0000 (UTC)"],"X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","From":"Nicolai Stange <nstange@suse.de>","To":"Bjorn Helgaas <helgaas@kernel.org>","Cc":"Greg Kroah-Hartman <gregkh@linuxfoundation.org>,\n\tNicolai Stange <nstange@suse.de>, Bjorn Helgaas <bhelgaas@google.com>,\n\tAdrian Salido <salidoa@google.com>, Sasha Levin <sasha.levin@oracle.com>,\n\tlinux-kernel@vger.kernel.org, linux-pci@vger.kernel.org","Subject":"Re: [PATCH 2/3] PCI: don't use snprintf() in driver_override_show()","References":"<20170911074542.16777-1-nstange@suse.de>\n\t<20170911074542.16777-3-nstange@suse.de>\n\t<20170911115511.GB16198@kroah.com>\n\t<20170925234819.GH15970@bhelgaas-glaptop.roam.corp.google.com>","Date":"Tue, 26 Sep 2017 08:36:11 +0200","In-Reply-To":"<20170925234819.GH15970@bhelgaas-glaptop.roam.corp.google.com>\n\t(Bjorn Helgaas's message of \"Mon, 25 Sep 2017 18:48:19 -0500\")","Message-ID":"<87zi9ivuec.fsf@suse.de>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux)","MIME-Version":"1.0","Content-Type":"text/plain","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<linux-pci.vger.kernel.org>","X-Mailing-List":"linux-pci@vger.kernel.org"}},{"id":1775825,"web_url":"http://patchwork.ozlabs.org/comment/1775825/","msgid":"<20170926200254.GP15970@bhelgaas-glaptop.roam.corp.google.com>","list_archive_url":null,"date":"2017-09-26T20:02:54","subject":"Re: [PATCH 2/3] PCI: don't use snprintf() in driver_override_show()","submitter":{"id":67298,"url":"http://patchwork.ozlabs.org/api/people/67298/","name":"Bjorn Helgaas","email":"helgaas@kernel.org"},"content":"On Tue, Sep 26, 2017 at 08:36:11AM +0200, Nicolai Stange wrote:\n> Bjorn Helgaas <helgaas@kernel.org> writes:\n> \n> > On Mon, Sep 11, 2017 at 04:55:11AM -0700, Greg Kroah-Hartman wrote:\n> >> On Mon, Sep 11, 2017 at 09:45:41AM +0200, Nicolai Stange wrote:\n> >> > Quote from Documentation/filesystems/sysfs.txt:\n> >> > \n> >> >   show() must not use snprintf() when formatting the value to be\n> >> >   returned to user space. If you can guarantee that an overflow\n> >> >   will never happen you can use sprintf() otherwise you must use\n> >> >   scnprintf().\n> >> > \n> >> > Commit 4efe874aace5 (\"PCI: Don't read past the end of sysfs\n> >> > \"driver_override\" buffer\") introduced such a snprintf() usage from\n> >> > driver_override_show() while at the same time tweaking\n> >> > driver_override_store() such that the write buffer can't ever get\n> >> > overflowed.\n> >> > \n> >> > Reasoning:\n> >> > Since aforementioned commit, driver_override_store() only accepts to be\n> >> > written buffers less than PAGE_SIZE - 1 in size.\n> >> > \n> >> > The then kstrndup()'ed driver_override string will be at most PAGE_SIZE - 1\n> >> > in length, including the trailing '\\0'.\n> >> > \n> >> > After the addition of a '\\n' in driver_override_show(), the result won't\n> >> > exceed PAGE_SIZE characters in length, again including the trailing '\\0'.\n> >> > \n> >> > Hence, snprintf(buf, PAGE_SIZE, ...) and sprintf(buf, ...) are equivalent\n> >> > at this point.\n> >> > \n> >> > Replace the former by the latter in order to adhere to the rules in\n> >> > Documentation/filesystems/sysfs.txt.\n> >> > \n> >> > This is a style fix only and there's no change in functionality.\n> >> > \n> >> > Signed-off-by: Nicolai Stange <nstange@suse.de>\n> >> > ---\n> >> >  drivers/pci/pci-sysfs.c | 2 +-\n> >> >  1 file changed, 1 insertion(+), 1 deletion(-)\n> >> > \n> >> > diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c\n> >> > index 8e075ea2743e..43f7fbede448 100644\n> >> > --- a/drivers/pci/pci-sysfs.c\n> >> > +++ b/drivers/pci/pci-sysfs.c\n> >> > @@ -722,7 +722,7 @@ static ssize_t driver_override_show(struct device *dev,\n> >> >  \tssize_t len;\n> >> >  \n> >> >  \tdevice_lock(dev);\n> >> > -\tlen = snprintf(buf, PAGE_SIZE, \"%s\\n\", pdev->driver_override);\n> >> > +\tlen = sprintf(buf, \"%s\\n\", pdev->driver_override);\n> >> \n> >> While I'm all for changes like this, it's an uphill battle to change\n> >> them, usually it's best to just catch them before they go into the tree.\n> \n> I did this patch mostly for demonstrating why the exclusion of the\n> sprintf() -> snprintf() change from the port of\n> \n>   3d713e0e382e (\"driver core: platform: add device binding path 'driver_override'\")\n> \n> to platform in [3/3] is safe. Because I'm pretty sure that [3/3] would\n> have been rejected if it had included that chunk -- due to the\n> non-conformity to Documentation/.\n> \n> \n> >> Anyway, nice summary, very good job with that.\n> >> \n> >> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>\n> >\n> > Why use snprintf() instead of scnprintf()?  \n> \n> s/snprintf/sprintf/, right?\n\nYep, sorry!  I meant \"why use sprintf() instead of scnprintf()?\"\n\n> > It looks like snprintf() is probably safe, but requires a bunch of\n> > analysis to prove that, while scnprintf() would be obviously safe.\n> \n> Personal preference, I guess. Something like \"using sprintf() would\n> implicitly document that the buffer is always large enough\", i.e. that\n> the size check in driver_override_store() is already strict enough / correct.\n> \n> Anyway, I don't have a strong opinion on that. So if you want me to\n> change this, I'll do, of course. Just note that Greg K-H. has queued up\n> [3/3] somewhere already and thus, it would probably require two patches\n> rather than only this one to make PCI and platform look the same again,\n> provided that's what is wanted.\n> \n> So, given that this patch has fulfilled its purpose already, I'm fine\n> with either of\n> - dropping it\n> - taking it\n> - s/sprintf/scnprintf/ and do the same for platform.\n\nOK, I think I'll drop it.\n\nI think the documentation is misleading.  The problem with\nsnprintf() is not that it might overflow the buffer; the problem is\nthat it might return more characters than it actually put in the\nbuffer.\n\nI would be happy to see patches that change driver_override_show()\n(there are three of them) to use scnprintf() instead of sprintf() or\nsnprintf().\n\nIs the third case (the amba driver_override store/show functions)\nsusceptible to the same issues you're fixing for platform and PCI?\n\nBjorn","headers":{"Return-Path":"<linux-pci-owner@vger.kernel.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-pci-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","mail.kernel.org;\n\tdmarc=none (p=none dis=none) header.from=kernel.org","mail.kernel.org;\n\tspf=none smtp.mailfrom=helgaas@kernel.org"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y1sM86q07z9t3R\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 27 Sep 2017 06:03:00 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S968628AbdIZUC6 (ORCPT <rfc822;incoming@patchwork.ozlabs.org>);\n\tTue, 26 Sep 2017 16:02:58 -0400","from mail.kernel.org ([198.145.29.99]:47594 \"EHLO mail.kernel.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S965209AbdIZUC5 (ORCPT <rfc822;linux-pci@vger.kernel.org>);\n\tTue, 26 Sep 2017 16:02:57 -0400","from localhost (unknown [69.71.4.159])\n\t(using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits))\n\t(No client certificate requested)\n\tby mail.kernel.org (Postfix) with ESMTPSA id 5D8D82170E;\n\tTue, 26 Sep 2017 20:02:55 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mail.kernel.org 5D8D82170E","Date":"Tue, 26 Sep 2017 15:02:54 -0500","From":"Bjorn Helgaas <helgaas@kernel.org>","To":"Nicolai Stange <nstange@suse.de>","Cc":"Greg Kroah-Hartman <gregkh@linuxfoundation.org>,\n\tBjorn Helgaas <bhelgaas@google.com>, Adrian Salido <salidoa@google.com>,\n\tSasha Levin <sasha.levin@oracle.com>,\n\tlinux-kernel@vger.kernel.org, linux-pci@vger.kernel.org","Subject":"Re: [PATCH 2/3] PCI: don't use snprintf() in driver_override_show()","Message-ID":"<20170926200254.GP15970@bhelgaas-glaptop.roam.corp.google.com>","References":"<20170911074542.16777-1-nstange@suse.de>\n\t<20170911074542.16777-3-nstange@suse.de>\n\t<20170911115511.GB16198@kroah.com>\n\t<20170925234819.GH15970@bhelgaas-glaptop.roam.corp.google.com>\n\t<87zi9ivuec.fsf@suse.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<87zi9ivuec.fsf@suse.de>","User-Agent":"Mutt/1.5.21 (2010-09-15)","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<linux-pci.vger.kernel.org>","X-Mailing-List":"linux-pci@vger.kernel.org"}}]