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

CA-406953: avoid pointer truncation and uninitialised value usage #19

Merged
merged 7 commits into from
Feb 21, 2025

Conversation

edwintorok
Copy link
Contributor

Looks like xha never got ported to 64-bit and still has a lot of 32-bit specific code.

When casting a pointer to integer we should use uintptr_t, which matches the size of a pointer (32-bit on 32-bit platforms, 64-bit on 64-bit platforms).

Otherwise we may lose the upper 32-bit of a pointer in hypercall arguments, which will likely cause the hypercall to fail.

Found by compiler warnings (GCC/Clang).

There are more compiler warnings that we should fix, but they are not so critical as this one.

@edwintorok edwintorok requested a review from andyhhp February 19, 2025 10:24
The assert here checks for alignment, so technically truncating the upper bits is not wrong,
but use the correct size.

Signed-off-by: Edwin Török <[email protected]>
Whether we logged on page allocation failures or not depended on `ret`
which was always uninitialized.
Choose not to log, because logging can delay us by an arbitrary amount,
and fencing is time sensitive.

Signed-off-by: Edwin Török <[email protected]>
cleanupwatchdog.c:240:32: warning: cast to smaller integer type 'unsigned int' from 'sched_watchdog_t *' (aka 'struct sched_watchdog *') [-Wpointer-to-int-cast]
  240 |     hypercall.arg[1] = (__u64) (unsigned int) &arg;  // pointer to u64
      |                                ^~~~~~~~~~~~~~~~~~~

Signed-off-by: Edwin Török <[email protected]>
Avoid these warnings:
```
statefileio.c:216:22: warning: cast to smaller integer type 'unsigned int' from 'struct _sf_global *' [-Wpointer-to-int-cast]
```

Signed-off-by: Edwin Török <[email protected]>
@edwintorok
Copy link
Contributor Author

I've included this commit 1d279d5 into this PR, because without it we get another pointer-to-int-cast (which is an error now). This one calculates an offset, and that one won't exceed 32-bit, but still better to avoid the truncation.

@edwintorok edwintorok merged commit 17e7f40 into xenserver:master Feb 21, 2025
4 checks passed
edwintorok added a commit to edwintorok/xha that referenced this pull request Feb 21, 2025
CA-406953: avoid pointer truncation and uninitialised value usage
edwintorok added a commit to edwintorok/xha that referenced this pull request Feb 21, 2025
CA-406953: avoid pointer truncation and uninitialised value usage
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.

3 participants