Re: kern/58425: negation of pid or pgid is UB for INT_MIN

[Robert Elz <kre%munnari.OZ.AU@localhost>]

The following reply was made to PR kern/58425; it has been noted by GNATS.

From: Robert Elz <kre%munnari.OZ.AU@localhost>
To: netbsd-bugs%netbsd.org@localhost, gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: kern/58425: negation of pid or pgid is UB for INT_MIN
Date: Sun, 14 Jul 2024 10:52:19 +0700

     Date:        Sat, 13 Jul 2024 19:55:00 +0000 (UTC)
     From:        campbell+netbsd%mumble.net@localhost
     Message-ID:  <20240713195500.A81291A923C%mollari.NetBSD.org@localhost>
 
   | This could be a theoretical issue or it could be a practical issue,
 
 I believe that unless we have some architecture which traps when
 executing -INT_MIN rather than just generating INT_MIN as the result
 (which is conceivable, but I doubt anything supported will do that)
 then I suspect this is really a non-issue for practical purposes.
 Even if the architecture were to generate a random value (which UB
 allows) it would not be truly harmful ... an application is allowed
 to send any random value to these sys calls, and things must work,
 that the random value was generated by this operation, rather than
 some other way really doesn't matter.
 
 It is possible for an application to do kill(INT_MIN, sig) and pass
 through INT_MIN as the pid, which being negative, is treated as a
 pgrp, negated.   But assuming -INT_MIN actually becomes -INT_MIN
 or something else, not a trap, that would be harmless, it simply results
 in a search for something different -- if it remains INT_MIN, the normal
 case for most 2's complement architectures, or anything else negative,
 it cannot be found, as all pids (and hence all pgrp ids) are positive
 (or 0 for the kernel) a negative value (any negative value) can never
 match (that's how this hack was invented).   So the result would have
 just been ESRCH anyway I believe.
 
 I am however in the process of validating the value, and not performing
 the undefined operation.   Note, that as (ktrace excepted, where the
 pid arg is actually defined to be an int) generally dealing with pid_t
 values, which we cannot assume are int ... it could be short (and once
 was, even earlier I think it might have been char, not that the name
 "pid_t" existed then, but that was a long long time ago) which would
 never give a problem, it gets promoted to int for both the -pid
 operation (and so could never be INT_MIN) and to pass as a parameter.
 Alternatively, pid_t might be wider than an int (could be long)
 in which case the value might be < INT_MIN, so I am making (most of)
 the tests check that the value is > INT_MIN not just != INT_MIN.
 
 My test build looks to have succeeded (on amd64) - currently building
 the sets - I will do an i386 build, and then commit the changes, and
 after a few says, see about pullups to -10 and -9.
 
 The only system place outside the kernel I could see was in the killpg()
 function (compat function, for old BSD code, posix defines the kill(-pid)
 method) - that one will be fixed as well.
 
 Random applications or other libraries which might accept a pid, and
 negate it, will need to wait for some other time (if there are any).
 
 kre
 
 ps: Taylor, if you feel we need ATF test cases for this, by all means,
 go ahead.