rust-fitsio icon indicating copy to clipboard operation
rust-fitsio copied to clipboard

Unaligned Pointer Read

Open petesmc opened this issue 2 years ago • 6 comments

This test using a i8 results in an unaligned pointer read...

https://github.com/simonrw/rust-fitsio/blob/1972db698dac94440f11e201eccbebe95ec9dace/fitsio/src/headers.rs#L246

... due to

&value as *const $t as *mut c_void, 

... in the below.

https://github.com/simonrw/rust-fitsio/blob/1972db698dac94440f11e201eccbebe95ec9dace/fitsio/src/headers.rs#L102-L125

Which calls ffpkyj in the cfitsio library like so:

ffpkyj(fptr, keyname, (LONGLONG) *(short *) value, comm, status); // <--- pointer to i8 gets converted into pointer to short.

I believe to fix this, you need to convert i8 -> i16 given you are using TSHORT, OR map i8 to TBYTE instead.

petesmc avatar Dec 05 '23 15:12 petesmc

Hi, thanks for reporting this issue, and I'm glad you're using the project! You are absolutely right, the types do not match up for u8.

Do you have a minimal example which I can use to reproduce this issue, and make sure it doesn't happen again?

simonrw avatar Dec 05 '23 15:12 simonrw

Sorry nothing I can share at the moment. I detected it because I'm rewriting part of cfitsio in rust which in debug mode checks for misaligned pointer reads.

petesmc avatar Dec 06 '23 13:12 petesmc

Ok thanks. I've merged in a fix and am ok to not include a test for now. Can you use the latest main commit and test for me please?

simonrw avatar Dec 06 '23 16:12 simonrw

@petesmc have you had a chance to test my fix?

simonrw avatar Apr 05 '24 16:04 simonrw

Looks good for bytes, but think you need to update the u16/i16 as well to TUSHORT/TSHORT

petesmc avatar Apr 11 '24 12:04 petesmc

@petesmc done, can you check again?

simonrw avatar Jul 15 '24 19:07 simonrw