PATCH: ctdb: buffer write beyond limits
swen at linux.ibm.com
Tue Feb 19 09:53:41 UTC 2019
On Tue, 2019-02-19 at 10:00 +0100, Volker Lendecke via samba-technical
> On Tue, Feb 19, 2019 at 09:26:17AM +0100, swen wrote:
> > On Mon, 2019-02-18 at 15:57 -0800, Jeremy Allison wrote:
> > > On Mon, Feb 18, 2019 at 06:22:49PM +0100, swen via samba-
> > > technical
> > > wrote:
> > > This is *way* too complex to even understand as a comment.
> > >
> > > Please just add the buffer overrun/overflow checks.
> > >
> > > That way we *KNOW* it's safe and don't need to read
> > > "War and Peace" to understand.
> > >
> > > NAK until then !
> > Next try...this time with explicit check.
> > Please review and push if happy
> No, this is not right. Please check every individual arithmetic
> operation in isolation. First do the "+". Put the result into a temp
> variable. Check that it did not overflow. Then either do the "-" and
> check that it did not underflow. Or check that the minuend is larger
> than the subtrahend and then do the subtraction. For example you
> take a look at tdb_expand_adjust() how we do these checks usually.
the code is right if the following facts are taken into account.
1. MAX_SIGNED_INT = MAX_UNSIGNED_INT/2 - 1
2. length is defined as unsigned int and will get assigned at most MAX_SIGNED_INT(num_ready)
3. offset is initially zero and will only grow as much as length is shrinking (=>code)
=> therefore, offset + length are at most MAX_SIGNED_INT. Guaranteed !
4. If then another (guaranteed positive) signed integer (here num_ready) is added we simply
cannot overflow an unsigned int because
MAX_SIGNED_INT + MAX_SIGNED_INT = MAX_UNSIGNED_INT - 2
Taken those simple maths into account, I hope you can agree to the code.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 833 bytes
Desc: This is a digitally signed message part
More information about the samba-technical