Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libspl sys/sysmacros.h: Fix P2ROUNDUP_TYPED to not trigger integer overflow #3949

Closed
wants to merge 1 commit into from

Conversation

perfinion
Copy link
Contributor

The original P2ROUNDUP_TYPED macro contains a -x which triggers PaX's
integer overflow detection for unsigned integers. Replace the macro with
an equivalent version that does not trigger the overflow.

Signed-off-by: Jason Zaman [email protected]

fixes #2505

@ryao
Copy link
Contributor

ryao commented Oct 26, 2015

After talking with @perfinion about this, I am confident that this is mathematically equivalent and can prove it. Here are axioms:

a. (-(x)) === (((x) - 1)) === ((x) + 1) under two's complement
b. (x & y) === (((x)) | (~(y))) under De Morgan's law
c. ~(~x) === x under the law of the excluded middle

Here is a proof:

  1. (-(-(x) & -(align)))
  2. (~(-(x) & -(align)) + 1) by a
  3. ((((-(x))) | ((-(align)))) + 1) by b
  4. ((((((x) - 1))) | ((((align) - 1)))) + 1) by a
  5. (((((x) - 1)) | (((align) - 1))) + 1) by c

Then if we just clean up the unnecessarily parenthesis, we arrive at @perfinion's code.

@behlendorf This looks good to me, provided the commit message includes the proof.

@perfinion
Copy link
Contributor Author

I updated the commit message after discussion with @ryao and included the proof from the issue. Also added Reviewed by @ryao and @chrisrd.

The original P2ROUNDUP and P2ROUNDUP_TYPED macros contain -x which
triggers PaX's integer overflow detection for unsigned integers.
Replace the macros with an equivalent version that does not trigger
the overflow.

Axioms:
A. (-(x)) === (~((x) - 1)) === (~(x) + 1) under two's complement.
B. ~(x & y) === ((~(x)) | (~(y))) under De Morgan's law.
C. ~(~x) === x under the law of excluded middle.

Proof:
0. (-(-(x) & -(align))) original
1. (~(-(x) & -(align)) + 1) by A
2. (((~(-(x))) | (~(-(align)))) + 1) by B
3. (((~(~((x) - 1))) | (~(~((align) - 1)))) + 1) by A
4. (((((x) - 1)) | (((align) - 1))) + 1) by C
Q.E.D.

Signed-off-by: Jason Zaman <[email protected]>
Reviewed-by: Chris Dunlop <[email protected]>
Reviewed-by: Richard Yao <[email protected]>
ryao pushed a commit to ryao/zfs that referenced this pull request Dec 3, 2015
The original P2ROUNDUP and P2ROUNDUP_TYPED macros contain -x which
triggers PaX's integer overflow detection for unsigned integers.
Replace the macros with an equivalent version that does not trigger
the overflow.

Axioms:
A. (-(x)) === (~((x) - 1)) === (~(x) + 1) under two's complement.
B. ~(x & y) === ((~(x)) | (~(y))) under De Morgan's law.
C. ~(~x) === x under the law of excluded middle.

Proof:
0. (-(-(x) & -(align))) original
1. (~(-(x) & -(align)) + 1) by A
2. (((~(-(x))) | (~(-(align)))) + 1) by B
3. (((~(~((x) - 1))) | (~(~((align) - 1)))) + 1) by A
4. (((((x) - 1)) | (((align) - 1))) + 1) by C
Q.E.D.

Signed-off-by: Jason Zaman <[email protected]>
Reviewed-by: Chris Dunlop <[email protected]>
Reviewed-by: Richard Yao <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#3949
goulvenriou pushed a commit to Alyseo/zfs that referenced this pull request Dec 8, 2015
The original P2ROUNDUP and P2ROUNDUP_TYPED macros contain -x which
triggers PaX's integer overflow detection for unsigned integers.
Replace the macros with an equivalent version that does not trigger
the overflow.

Axioms:
A. (-(x)) === (~((x) - 1)) === (~(x) + 1) under two's complement.
B. ~(x & y) === ((~(x)) | (~(y))) under De Morgan's law.
C. ~(~x) === x under the law of excluded middle.

Proof:
0. (-(-(x) & -(align))) original
1. (~(-(x) & -(align)) + 1) by A
2. (((~(-(x))) | (~(-(align)))) + 1) by B
3. (((~(~((x) - 1))) | (~(~((align) - 1)))) + 1) by A
4. (((((x) - 1)) | (((align) - 1))) + 1) by C
Q.E.D.

Signed-off-by: Jason Zaman <[email protected]>
Reviewed-by: Chris Dunlop <[email protected]>
Reviewed-by: Richard Yao <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#3949
@perfinion perfinion deleted the pax branch December 18, 2015 03:53
nedbass pushed a commit that referenced this pull request Dec 24, 2015
The original P2ROUNDUP and P2ROUNDUP_TYPED macros contain -x which
triggers PaX's integer overflow detection for unsigned integers.
Replace the macros with an equivalent version that does not trigger
the overflow.

Axioms:
A. (-(x)) === (~((x) - 1)) === (~(x) + 1) under two's complement.
B. ~(x & y) === ((~(x)) | (~(y))) under De Morgan's law.
C. ~(~x) === x under the law of excluded middle.

Proof:
0. (-(-(x) & -(align))) original
1. (~(-(x) & -(align)) + 1) by A
2. (((~(-(x))) | (~(-(align)))) + 1) by B
3. (((~(~((x) - 1))) | (~(~((align) - 1)))) + 1) by A
4. (((((x) - 1)) | (((align) - 1))) + 1) by C
Q.E.D.

Signed-off-by: Jason Zaman <[email protected]>
Reviewed-by: Chris Dunlop <[email protected]>
Reviewed-by: Richard Yao <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #3949
goulvenriou pushed a commit to Alyseo/zfs that referenced this pull request Feb 4, 2016
The original P2ROUNDUP and P2ROUNDUP_TYPED macros contain -x which
triggers PaX's integer overflow detection for unsigned integers.
Replace the macros with an equivalent version that does not trigger
the overflow.

Axioms:
A. (-(x)) === (~((x) - 1)) === (~(x) + 1) under two's complement.
B. ~(x & y) === ((~(x)) | (~(y))) under De Morgan's law.
C. ~(~x) === x under the law of excluded middle.

Proof:
0. (-(-(x) & -(align))) original
1. (~(-(x) & -(align)) + 1) by A
2. (((~(-(x))) | (~(-(align)))) + 1) by B
3. (((~(~((x) - 1))) | (~(~((align) - 1)))) + 1) by A
4. (((((x) - 1)) | (((align) - 1))) + 1) by C
Q.E.D.

Signed-off-by: Jason Zaman <[email protected]>
Reviewed-by: Chris Dunlop <[email protected]>
Reviewed-by: Richard Yao <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#3949
@behlendorf behlendorf added this to the 0.6.5.4 milestone Mar 23, 2016
@ryao ryao mentioned this pull request Mar 20, 2023
25 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PAX: size overflow detected in function zil_itx_create
3 participants