2013-05-26 08:51:18

by Chen Gang

[permalink] [raw]
Subject: [PATCH] arch: x86: platform: mrst: pr_err() need notice that 'sdev->modalias' may not a NUL terminated string.


According to the areas which use "%16.16s" within this file, the
'sdev->modalias' may not always be a NUL terminated string.

So need use "%.16s" instead of "%s" in pr_err(), or may cause issue.

Signed-off-by: Chen Gang <[email protected]>
---
arch/x86/platform/mrst/mrst.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/platform/mrst/mrst.c b/arch/x86/platform/mrst/mrst.c
index a0a0a43..e2e069c 100644
--- a/arch/x86/platform/mrst/mrst.c
+++ b/arch/x86/platform/mrst/mrst.c
@@ -746,7 +746,7 @@ static void __init intel_scu_spi_device_register(struct spi_board_info *sdev)

new_dev = kzalloc(sizeof(*sdev), GFP_KERNEL);
if (!new_dev) {
- pr_err("failed to alloc mem for delayed spi dev %s\n",
+ pr_err("failed to alloc mem for delayed spi dev %.16s\n",
sdev->modalias);
return;
}
--
1.7.7.6


Subject: Re: [PATCH] arch: x86: platform: mrst: pr_err() need notice that 'sdev->modalias' may not a NUL terminated string.

On Sun, May 26, 2013 at 04:50:22PM +0800, Chen Gang wrote:
>
> According to the areas which use "%16.16s" within this file, the
> 'sdev->modalias' may not always be a NUL terminated string.
>
> So need use "%.16s" instead of "%s" in pr_err(), or may cause issue.

No. spi_match_id() expects a NULL terminated string and I would be suprised if
it would be otherwise. Therefore moorsetown or this files has to learn to NULL
terminate its strings.

> Signed-off-by: Chen Gang <[email protected]>

Sebastian

2013-05-27 08:17:34

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] arch: x86: platform: mrst: pr_err() need notice that 'sdev->modalias' may not a NUL terminated string.

On 05/27/2013 03:06 PM, Sebastian Andrzej Siewior wrote:
> On Sun, May 26, 2013 at 04:50:22PM +0800, Chen Gang wrote:
>> >
>> > According to the areas which use "%16.16s" within this file, the
>> > 'sdev->modalias' may not always be a NUL terminated string.
>> >
>> > So need use "%.16s" instead of "%s" in pr_err(), or may cause issue.
> No. spi_match_id() expects a NULL terminated string and I would be suprised if
> it would be otherwise. Therefore moorsetown or this files has to learn to NULL
> terminate its strings.
>

In spi_match_id(), it is 'spi_device', not 'spi_board_info'. ;-)

But all together, it is my fault, the original implementation has no
issue: "I treated SFI_NAME_LEN as SPI_NAME_LEN when strncpy()".

BTW: 'pentry->name' in sfi_parse_devs() may not always be a NUL
terminated string, could you give a check, too ? ;-)


Thanks.
--
Chen Gang

Asianux Corporation