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

Fix ubsan complaint about incorrect left-shift in generate_local_port() #379

Closed

Conversation

ievenbach
Copy link
Contributor

The value of "n" used to calculate get the unique port within application is 10 bit.
This change applies corresponding mask to n, prior to left-shifting it.

@thom311
Copy link
Owner

thom311 commented Apr 24, 2024

I am not sure that this is right. n & 0x3ff is a positive signed integer of at most 0x3ff. If you shift that signed value by 22 bits to the left, then the value can become negative, which is again undefined behavior. I think, this would require at least n & 0x3ffu.

I think the problem here is that shifting a u16 value by 22 is undefined behavior. If so, a better fix is ((uint32_t) n) << 22).

Does that also avoid the ubsan issue?

n needs to be uint32_t to fit left shift by 22 bits
@ievenbach ievenbach force-pushed the aurora/limit-lshift-value-socket_c branch from 4186c1d to 98a6f29 Compare April 24, 2024 20:26
@ievenbach
Copy link
Contributor Author

OK. You are right. I simply changed data type of n, and it worked. Is there a good reason for n to still be uint16_t?

thom311 pushed a commit that referenced this pull request Apr 26, 2024
…cal_port()

n needs to be uint32_t to fit left shift by 22 bits

#379
@thom311
Copy link
Owner

thom311 commented Apr 26, 2024

merged. Thank you!!

@thom311 thom311 closed this Apr 26, 2024
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.

2 participants