[v2] sprof: check pread size and offset for overflow

[Collin Funk]

DJ Delorie <dj@redhat.com> writes:

> Paul Eggert <eggert@cs.ucla.edu> writes:
>
>> On 2025-10-16 12:23, DJ Delorie wrote:
>>> +#define PCHECK(sz,off) if ((sz) > st.st_size				\
>>> +			   || (off_t)(off) < 0 || (off_t)(off) > st.st_size \
>>> +			   ((sz)+(off_t)(off)) > st.st_size)		\
>>
>> This still could have undefined behavior, as the addition in the last 
>> line could have signed integer overflow.
>
> It's a problem only if the file is bigger than SSIZE_T_MAX, else the
> other checks would have failed, and we can't support files that big
> anyway (see below).
>
> The core problem that triggered this is that the offset value is read as
> an unsigned, but passed as a signed, and making sure that doesn't result
> in a negative value passed to pread() is a key part of this.
>
>> What you're trying to say here, is that SZ and OFF are both nonnegative, 
>> and that their sum is less than ST.st_size.
>
> AND that the values we read from the ELF file are reasonable.  That
> includes checking each against st.st_size.  We want paranoia, not
> efficiency.
>
> GIVEN we should check that anyway, I think that removes the UB in the
> addition, aside from files bigger than SSIZE_T_MAX, but for such a file,
> we can't address the whole thing anyway - pread() would error due to the
> signed off_t type of the offset parameter.

In Coreutils, Paul and I have written stuff like this:

    intmax_t imax_uid = /* some value read from command-line ...  */;
    uid_t uid;
    /* Or __builtin_add_overflow (imax_uid, 0, &uid)  */
    if (ckd_add (&uid, imax_uid, 0))
      {
         error (EXIT_FAILURE, ...);
      }

To make sure that a value fits in an integer type of an unspecified
width, like uid_t or ssize_t.

Generally, I prefer to avoid writing overflow checks manually since it
is easier to introduce bugs than the ckd_*/__builtin_*_overflow
functions. And the checked functions are shorter/more clear.

Collin