svn commit: r311109 - head/usr.bin/patch

[Pedro Giffuni]

On 01/03/17 11:04, Konstantin Belousov wrote:
> On Tue, Jan 03, 2017 at 08:52:03AM -0700, Ian Lepore wrote:
>> On Tue, 2017-01-03 at 12:26 +0200, Konstantin Belousov wrote:
>>> On Mon, Jan 02, 2017 at 07:41:26PM -0500, Pedro Giffuni wrote:
>>>>
>>>>
>>>>
>>>> On 01/02/17 17:54, Conrad Meyer wrote:
>>>>>
>>>>> I was suggesting using UINT32_MAX/2 on all platforms (which is
>>>>> safe
>>>>> everywhere).
>>>>>
>>>> Ah OK. INT_MAX is ~ (UINT_MAX / 2) so it's the same to use either.
>>>> I just think it's clearer to use INT_MAX and the corresponding int
>>>> type.
>>>>
>>>> The other issue is if diff(1) can handle such lines(?).
>>> Of course it cannot, on ILP32 arches.
>>>
>>
>> I kind of don't understand the premise of the naysayers in this thread.
>>  Some machines cannot do lines that are UINT_MAX long, so in that case
>> we should not support any lines longer than USHORT_MAX?  As if there
>> aren't *billions* of line length limits to choose from between those
>> two numbers?
>>
>> I'm also trying to picture the real-world need to diff and patch lines
>> of ascii text longer than 64K, but for every problem out there, there
>> is someone with a perverse need to solve that problem outside of the
>> normal lines we all live between.
>
> The original disallow of lines longer than USHORT_MAX can be reasonably
> interpreted as an attempt to only process patches which have sensible
> data. The test for UINT_MAX or INT_MAX have no sense at all: what
> should that check prevent ? On 32bit arches, malloc(3) is guaranteed
> to fail for such large requests (for typical memory maps, malloc(3)
> would be unable to allocate even 1G), for 64bit arches this is still an
> artificial limitation.
>

I think this is a valid reason to leave things as they are. FWIW, both
NetBSD and OpenBSD seem be fine with just SHORT_MAX.
Given the default width in GNU diff is 130, exceeding USHORT_MAX means 
the tool is not being used for code.

As you say, another question is if we should be limiting the use of 
patch(1) for code only.


> Might be this is an attempt to provide DoS mitigation ?  It is
> probably too naive for that.
>
> In other words, I do see some reasoning in UINT_SHORT limit, is it useful
> goal or not, is another question. While the check for INT_MAX does not
> provide any value, IMO.
>

The check is basically a post-mortem attempt, it is of little relevance
and the other BSDs don't have it.

Pedro.