* [PATCH v3] staging: android: Replace strcpy with strlcpy
@ 2017-03-11 22:02 simran singhal
2017-03-12 13:34 ` Greg KH
0 siblings, 1 reply; 4+ messages in thread
From: simran singhal @ 2017-03-11 22:02 UTC (permalink / raw)
To: gregkh; +Cc: arve, riandrews, devel, linux-kernel, outreachy-kernel
Replace strcpy with strlcpy as strcpy does not check for buffer
overflow.
This is found using Flawfinder.
Signed-off-by: simran singhal <singhalsimran0@gmail.com>
---
v3:
-Correcting the place of the parenthesis and sign
drivers/staging/android/ashmem.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index 7cbad0d..4d9bf48 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -548,7 +548,8 @@ static int set_name(struct ashmem_area *asma, void __user *name)
if (unlikely(asma->file))
ret = -EINVAL;
else
- strcpy(asma->name + ASHMEM_NAME_PREFIX_LEN, local_name);
+ strlcpy(asma->name + ASHMEM_NAME_PREFIX_LEN, local_name,
+ sizeof(asma->name) - ASHMEM_NAME_PREFIX_LEN);
mutex_unlock(&ashmem_mutex);
return ret;
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] staging: android: Replace strcpy with strlcpy
2017-03-11 22:02 [PATCH v3] staging: android: Replace strcpy with strlcpy simran singhal
@ 2017-03-12 13:34 ` Greg KH
2017-03-12 14:55 ` SIMRAN SINGHAL
0 siblings, 1 reply; 4+ messages in thread
From: Greg KH @ 2017-03-12 13:34 UTC (permalink / raw)
To: simran singhal; +Cc: devel, outreachy-kernel, arve, riandrews, linux-kernel
On Sun, Mar 12, 2017 at 03:32:44AM +0530, simran singhal wrote:
> Replace strcpy with strlcpy as strcpy does not check for buffer
> overflow.
Can there be a buffer overflow here? If not, then strcpy is just fine
to use. Do you see a potential code path here that actually is a
problem using this?
> This is found using Flawfinder.
You mean 'grep'? :)
If not, what exactly does "Flawfinder" point out is wrong with the code
here? At first glance, I can't find it, but perhaps the tool, and your
audit, provided more information?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] staging: android: Replace strcpy with strlcpy
2017-03-12 13:34 ` Greg KH
@ 2017-03-12 14:55 ` SIMRAN SINGHAL
2017-03-12 15:04 ` Greg KH
0 siblings, 1 reply; 4+ messages in thread
From: SIMRAN SINGHAL @ 2017-03-12 14:55 UTC (permalink / raw)
To: Greg KH; +Cc: devel, outreachy-kernel, arve, riandrews, linux-kernel
On Sun, Mar 12, 2017 at 7:04 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Sun, Mar 12, 2017 at 03:32:44AM +0530, simran singhal wrote:
>> Replace strcpy with strlcpy as strcpy does not check for buffer
>> overflow.
>
> Can there be a buffer overflow here? If not, then strcpy is just fine
> to use. Do you see a potential code path here that actually is a
> problem using this?
>
>> This is found using Flawfinder.
>
> You mean 'grep'? :)
>
> If not, what exactly does "Flawfinder" point out is wrong with the code
> here? At first glance, I can't find it, but perhaps the tool, and your
> audit, provided more information?
>
> thanks,
>
Flawfinder reports possible security weaknesses (“flaws”) sorted by risk level.
The risk level is shown inside square brackets and varies from 0, very
little risk,
to 5, great risk.
So, here in this case I was getting risk of [4].
This is what I got:
drivers/staging/android/ashmem.c:551: [4] (buffer) strcpy:
Does not check for buffer overflows when copying to destination (CWE-120).
Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy is easily
misused).
> greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] staging: android: Replace strcpy with strlcpy
2017-03-12 14:55 ` SIMRAN SINGHAL
@ 2017-03-12 15:04 ` Greg KH
0 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2017-03-12 15:04 UTC (permalink / raw)
To: SIMRAN SINGHAL; +Cc: devel, outreachy-kernel, arve, riandrews, linux-kernel
On Sun, Mar 12, 2017 at 08:25:28PM +0530, SIMRAN SINGHAL wrote:
> On Sun, Mar 12, 2017 at 7:04 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Sun, Mar 12, 2017 at 03:32:44AM +0530, simran singhal wrote:
> >> Replace strcpy with strlcpy as strcpy does not check for buffer
> >> overflow.
> >
> > Can there be a buffer overflow here? If not, then strcpy is just fine
> > to use. Do you see a potential code path here that actually is a
> > problem using this?
> >
> >> This is found using Flawfinder.
> >
> > You mean 'grep'? :)
> >
> > If not, what exactly does "Flawfinder" point out is wrong with the code
> > here? At first glance, I can't find it, but perhaps the tool, and your
> > audit, provided more information?
> >
> > thanks,
> >
>
> Flawfinder reports possible security weaknesses (“flaws”) sorted by risk level.
> The risk level is shown inside square brackets and varies from 0, very
> little risk,
> to 5, great risk.
>
> So, here in this case I was getting risk of [4].
> This is what I got:
> drivers/staging/android/ashmem.c:551: [4] (buffer) strcpy:
> Does not check for buffer overflows when copying to destination (CWE-120).
> Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy is easily
> misused).
Consider looking at the code to see if it actually is incorrect before
blindly accepting random comments by a random tool :)
Again, if you can see how this is incorrect, great, let's fix it,
otherwise please leave it as-is because so far your fixes are actually
breaking things :(
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-03-12 15:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-11 22:02 [PATCH v3] staging: android: Replace strcpy with strlcpy simran singhal
2017-03-12 13:34 ` Greg KH
2017-03-12 14:55 ` SIMRAN SINGHAL
2017-03-12 15:04 ` Greg KH
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).