linux-kernel - Re: [PATCH] scsi: hpsa: fix multiple issues in path_info

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]

Message-ID: <87mvuga9jr.fsf@rasmusvillemoes.dk>
Date:	Sat, 14 Nov 2015 16:24:24 +0100
From:	Rasmus Villemoes <linux@...musvillemoes.dk>
To:	Don Brace <brace77070@...il.com>
Cc:	Joe Handzik <joseph.t.handzik@...com>,
	"James E.J. Bottomley" <JBottomley@...n.com>,
	Kevin Barnett <kevin.barnett@...s.com>,
	Scott Teel <scott.teel@...s.com>,
	Tomas Henzl <thenzl@...hat.com>, iss_storagedev@...com,
	storagedev@...s.com, linux-scsi@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] scsi: hpsa: fix multiple issues in path_info_show

On Wed, Oct 28 2015, Don Brace <brace77070@...il.com> wrote:

> On 10/27/2015 05:16 PM, Rasmus Villemoes wrote:
>> I'm not familiar with this code, but path_info_show() (added in
>> 8270b86243658 "hpsa: add sysfs entry path_info to show box and
>> bay information") seems to be broken in multiple ways.
>>
[snip]
>>
>> We can fix all of that and get rid of the 400 byte stack buffer by
>> simply writing directly to the given output buffer, which the upper
>> layer guarantees is at least PAGE_SIZE. s[c]nprintf doesn't care where
>> it is writing to, so this doesn't make the spin lock hold time any
>> longer. Using scnprintf ensures that output_len always represents the
>> number of bytes actually written to the buffer, so we'll report the
>> proper amount to the upper layer.
>>
>> Signed-off-by: Rasmus Villemoes <linux@...musvillemoes.dk>
>>
> Thanks, I added this to my current patch set. This patch will be up
> with you as the author soon.

I see it in mainline now. May I ask why 6 out of 7 scnprintfs were
changed (back) to snprintf? I don't think there's any functional
difference as long as PAGE_SIZE is indeed sufficient, but mixing
snprintf and scnprintf is pretty odd, and there's now a discrepancy
between the commit log and the patch which wasn't in my original - I'd
expect a "[use snprintf because xyz]" note added if the change was
intentional.

Rasmus

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/