RFC: chgsbsize doesn't handle -ve sbsize correctly

Anuranjan Shukla anshukla at juniper.net
Fri Apr 3 07:39:10 UTC 2015


Hi,
In struct uidinfo{}, ui_sbsize member is an int. In a system with large
number of sockets owned by a user it's possible for ui_sbsize to roll over
to a negative value. When that happens, further calls to _increase_ sbsize
don't detect this condition as they should per design. But when the
sockets start shutting down this condition would be detected and you'll
see the console prints ("negative sbsize for uid =") happen. As a worst
case the console being flooded with these prints can create a DoS scenario
(specially on single CPU systems).

Regardless of the end result, the code itself needs to be fixed for
correctness. There are two things to note:
- Int doesn't seem to be the correct type for ui_sbsize. Should be a u_int
atleast.
- Since there's no real prevention of the overflow as it happens, the
warning print isn't helpful and should be a DEBUG level log at best. If we
change ui_sbsize to be a u_int, the negative check itself can go.


int
chgsbsize(uip, hiwat, to, max)
{
        int diff;

        diff = to - *hiwat;
        if (diff > 0) {
                if (atomic_fetchadd_long(&uip->ui_sbsize, (long)diff) +
diff > max) {
        <======= -ve ui_sbsize goes undetected and we'll pass the above
check 
                        atomic_subtract_long(&uip->ui_sbsize, (long)diff);
                        return (0);
                }
        } else {
                atomic_add_long(&uip->ui_sbsize, (long)diff);
                if (uip->ui_sbsize < 0)
                        printf("negative sbsize for uid = %d\n",
uip->ui_uid);

                <==== Now we'll detect, far too late, that sbsize went -ve



More information about the freebsd-arch mailing list