于 2012年11月09日 00:33, Sam Ravnborg 写道:
> Hi Cheng.
> On Thu, Nov 08, 2012 at 11:41:39AM +0800, Chen Gang wrote:
>>
>> when cp >= barg_buf + BARG_LEN-2, it only break internel loop (while)
>> but outside loop (for) still has effect, and "*cp++ = ' '" repeating
>> so need additional checking for it.
>>
>>
>> Signed-off-by: Chen Gang <gang.chen@xxxxxxxxxxx>
>
> I wonder how you found this bug?!?!
I only find it through "code review".
A) I grep all "strcpy" in kernel wide (about 2943 lines)
B) I check them one by one.
i) when I check strcpy, also reading all relative source code.
ii) my goal is finding issues.
iii) if I think it is valuable to continue reading, I will do.
C) when find arch/sparc, I find this bug (although it is not relative with strcpy).
It seems just starting (I have only finished checking 10 lines strcpy of 2943 lines),
> Anyway please consider this alternative fix:
>
> diff --git a/arch/sparc/prom/bootstr_32.c b/arch/sparc/prom/bootstr_32.c
> index f5ec32e..4ce602f 100644
> --- a/arch/sparc/prom/bootstr_32.c
> +++ b/arch/sparc/prom/bootstr_32.c
> @@ -31,14 +31,10 @@ prom_getbootargs(void)
> arg = (*(romvec->pv_v0bootargs))->argv[iter];
> if (arg == NULL)
> break;
> - while(*arg != 0) {
> - /* Leave place for space and null. */
> - if(cp >= barg_buf + BARG_LEN-2){
> - /* We might issue a warning here. */
> - break;
> - }
> + while (*arg != 0 && cp < (barg_buf + BARG_LEN - 2))
> *cp++ = *arg++;
> - }
> +
> + /* Append trailing space + null */
> *cp++ = ' ';
> }
> *cp = 0;
>
>
> Adding the conditional inside the while loop makes
> the logic simpler. And the patch actually deletes more lines than it adds.
> And please take care to follow coding style too. In particular spaces around operators.
>
> The old code does not follow coding style - but this is no excuse.
>
I agree with the contents above.
> Note - the above is not even build tested!
>
A) I think, it will be better to give a test (although it seems obviously)
B) but sorry for I have no relative environments now.
i) if testing is necessary;
ii) also if you have no environments, either.
iii) I should try. (please tell me)
> If you use the above code-snippet you can add my:
> Acked-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
>
I think it is necessary, you can do it, automatically.
if truly need I do, please tell me.
--
Chen Gang
Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
