* [PATCH V3] Fix buffer size warning for strcpy
@ 2021-08-23 14:35 Nigel Croxon
2021-08-23 15:36 ` Tkaczyk, Mariusz
0 siblings, 1 reply; 3+ messages in thread
From: Nigel Croxon @ 2021-08-23 14:35 UTC (permalink / raw)
To: jes, mariusz.tkaczyk, neilb, xni, linux-raid, gal.ofri
To meet requirements of Common Criteria certification vulnerability
assessment. Static code analysis has been run and found the following
error:
buffer_size_warning: Calling "strncpy" with a maximum size
argument of 16 bytes on destination array "ve->name" of
size 16 bytes might leave the destination string unterminated.
The change is to make the destination size to fit the allocated size.
V3: Doc change only:
The code change from filling ve->name with spaces to filling it with
null-terminated is to comform to the SNIA - Common RAID Disk Data
Format Specification. The format for VD_Name (ve->name) specifies
the field to be either ASCII or UNICODE. Bit 2 of the VD_Type field
MUST be used to determine the Unicode or ASCII format of this field.
If this field is not used, all bytes MUST be set to zero.
V2: Change from zero-terminated to zero-padded on memset and
change from using strncpy to memcpy, feedback from Neil Brown.
Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
---
super-ddf.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/super-ddf.c b/super-ddf.c
index dc8e512..1771316 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -2637,9 +2637,13 @@ static int init_super_ddf_bvd(struct supertype *st,
ve->init_state = DDF_init_not;
memset(ve->pad1, 0xff, 14);
- memset(ve->name, ' ', 16);
- if (name)
- strncpy(ve->name, name, 16);
+ memset(ve->name, '\0', sizeof(ve->name));
+ if (name) {
+ int l = strlen(ve->name);
+ if (l > 16)
+ l = 16;
+ memcpy(ve->name, name, l);
+ }
ddf->virt->populated_vdes =
cpu_to_be16(be16_to_cpu(ddf->virt->populated_vdes)+1);
--
2.29.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH V3] Fix buffer size warning for strcpy
2021-08-23 14:35 [PATCH V3] Fix buffer size warning for strcpy Nigel Croxon
@ 2021-08-23 15:36 ` Tkaczyk, Mariusz
2021-08-24 7:35 ` Tkaczyk, Mariusz
0 siblings, 1 reply; 3+ messages in thread
From: Tkaczyk, Mariusz @ 2021-08-23 15:36 UTC (permalink / raw)
To: Nigel Croxon, linux-raid
On 23.08.2021 16:35, Nigel Croxon wrote:
> To meet requirements of Common Criteria certification vulnerability
> assessment. Static code analysis has been run and found the following
> error:
> buffer_size_warning: Calling "strncpy" with a maximum size
> argument of 16 bytes on destination array "ve->name" of
> size 16 bytes might leave the destination string unterminated.
>
Yeah, please ignore my comment to v2- the task here it remove error, not
to acknowledge it.
> + int l = strlen(ve->name);
> + if (l > 16)
> + l = 16;
I think that whole "if" statement can be replaced by:
strnlen(ve->name, sizeof(ve->name))
> + memcpy(ve->name, name, l);
> + }
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH V3] Fix buffer size warning for strcpy
2021-08-23 15:36 ` Tkaczyk, Mariusz
@ 2021-08-24 7:35 ` Tkaczyk, Mariusz
0 siblings, 0 replies; 3+ messages in thread
From: Tkaczyk, Mariusz @ 2021-08-24 7:35 UTC (permalink / raw)
To: Nigel Croxon, linux-raid
Hello Nigel,
It current form it is not working. See my finding below.
On 23.08.2021 17:36, Tkaczyk, Mariusz wrote:
>
>> + int l = strlen(ve->name);
i think that you want to use name instead of ve->name.
length of ve->name is zero after memset.
>> + if (l > 16)
>> + l = 16;
> I think that whole "if" statement can be replaced by:
> strnlen(ve->name, sizeof(ve->name))
I did a mistake here.
I want to suggest usage of:
l = strnlen(name, sizeof(ve->name));
>> + memcpy(ve->name, name, l);
>> + }
>
Thanks,
Mariusz
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-08-24 7:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-23 14:35 [PATCH V3] Fix buffer size warning for strcpy Nigel Croxon
2021-08-23 15:36 ` Tkaczyk, Mariusz
2021-08-24 7:35 ` Tkaczyk, Mariusz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).