Re: [PATCH v3] n_tty: Fix buffer overruns with larger-than-4k pastes

[Peter Hurley <peter@hurleysoftware.com>]

On 12/09/2013 11:26 AM, Stas Sergeev wrote:
> 09.12.2013 18:50, Peter Hurley пишет:
>>   	if (found && read_buf(ldata, eol) == __DISABLED_CHAR) {
>>   		n--;
>>   		eof_push = !n && ldata->read_tail != ldata->line_start;
>> +		ldata->push = 0;
>>   	}
> Will this work if the last (and only) char written in raw
> mode appear to be \0 (__DISABLED_CHAR)?

That would have triggered an EOF in older kernels so not a
regression.

>
>> -		ldata->line_start = ldata->read_tail;
>> +		if (!ldata->push)
>> +			ldata->line_start = ldata->read_tail;
>> +		else
>> +			ldata->push = 0;
> Will this work if more that one "fake" EOF is accumulated
> in bitmap because of multiple icanon switches?

Not possible. The read_flags and push indicator are reset
with icanon -> !icanon switches.

> Also, I am a bit surprised with the presence of the code
> like this:
> ---
>
> |	if  (n>  4096)
> 		n+=  4096;
> |
>
> ---
> Am I the only one thinking it is unclear what it does?
> Doesn't it deserve the comment at least?
>
> Or this:
> ---
>
> |eof_push= !n&&  ldata->read_tail!=  ldata->line_start;|
>
> ---
> If eof_push means that the EOL mark was found not at the
> line start, then it is completely confusing why !n is here
> (one have to read a lot of context to find out why).|||
> When I created the patch, the code was much more easy
> to follow than now.

The previous kernels did byte-by-byte copy with lock/unlock
for every byte, into the input buffer and back out of the input
buffer. Simple, but inefficient.

This version has roughly 4n-2 fewer bus locks. The necessary
unsigned arithmetic and additional condition checks make the
code somewhat more complex.

But please feel free to submit patches that fix or clarify
anything you find.

Regards,
Peter Hurley