On Mon, Mar 13, 2017 at 6:27 PM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > On Mon, Mar 13, 2017 at 06:17:22PM +0530, SIMRAN SINGHAL wrote: >> On Mon, Mar 13, 2017 at 6:11 PM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: >> > On Sun, Mar 12, 2017 at 02:10:01AM +0530, simran singhal wrote: >> >> Replace strcpy with strlcpy as strcpy does not check for buffer >> >> overflow. >> >> This is found using Flawfinder. >> >> >> >> Signed-off-by: simran singhal <singhalsimran0@xxxxxxxxx> >> >> --- >> >> 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..eb2f4ef 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)); >> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >> > This isn't right. >> > >> > Also please do some analysis to see if it's a real bug or a false >> > positive. It is a false positive in this case. >> > >> >> Dan, >> I have already sent v3 of this in which I have used: >> sizeof(asma->name) - ASHMEM_NAME_PREFIX_LEN > > Yeah. I saw that. It's fine, I suppose but you should have done more > analysis to see if it was a real bug like Al and Greg suggested. The > changelog should say something like: > > "The destination buffer is 12345 bytes long but we're copying a 10000 > character string so it can overflow." Occasionally, I will fudge a > little bit on these changelogs to say that I have looked every where to > determine the size of the source buffer and can't figure it out so this > change makes it easier to audit. But I try to figure it out generally. > > Really tools should be able to show that this code is safe. They > currently don't so far as I know, but they should. It's a matter of > waiting a year for Smatch to improve. > Thanks! Will keep this in mind. > regards, > dan carpenter > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
