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.

Sign up for GitHub

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

Jump to bottom

Using Undefined Behavior to detect pointer arithmetic overflow in older version of libfdt #1967

Closed
viennadd opened this issue Nov 23, 2017 · 3 comments · Fixed by #1969
Closed

Using Undefined Behavior to detect pointer arithmetic overflow in older version of libfdt #1967

viennadd opened this issue Nov 23, 2017 · 3 comments · Fixed by #1969

Comments

@viennadd
Copy link
Contributor

viennadd commented Nov 23, 2017 •
edited
Loading

Hi all,

Our code scanner Pinpoint has reported an invalid testing for overflow in libfdt,

optee_os/core/lib/libfdt/fdt.c

Lines 77 to 91 in b1469ba

const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len)
{
const char *p;
if (fdt_version(fdt) >= 0x11)
if (((offset + len) < offset)
|| ((offset + len) > fdt_size_dt_struct(fdt)))
return NULL;
p = _fdt_offset_ptr(fdt, offset);
if (p + len < p)
return NULL;
return p;
}

the following overflow detection is undefined behavior and might be discarded by compilers in different optimization levels[1].

	if (p + len < p)
		return NULL;

this has been fixed in the upstream of libfdt:
dgibson/dtc@d0b3ab0#diff-842629fc73576aaea15c3de45cb95f93

Regards,
Alex, Sourcebrella Inc.

[1] Figure 4, Towards Optimization-Safe Systems: Analyzing the Impact of Undefined Behavior
image
jenswi-linaro added a commit to jenswi-linaro/optee_os that referenced this issue Nov 23, 2017
@jenswi-linaro
libfdt: fix undefined behaviour in fdt_offset_ptr()
a679e15 
Upstream commit d0b3ab0a0f46 ("libfdt: Fix undefined behaviour in
fdt_offset_ptr()").

Using pointer arithmetic to generate a pointer outside a known object is,
technically, undefined behaviour in C.  Unfortunately, we were using that
in fdt_offset_ptr() to detect overflows.

To fix this we need to do our bounds / overflow checking on the offsets
before constructing pointers from them.

Fixes: OP-TEE#1967
Signed-off-by: Jens Wiklander <[email protected]>
@jenswi-linaro jenswi-linaro mentioned this issue Nov 23, 2017
Merged
@jenswi-linaro
Copy link
Contributor

Fix in #1969

@jenswi-linaro
Copy link
Contributor

By the way, thanks for reporting. :-)

@viennadd
Copy link
Contributor Author

seems I can close this issue now.
thanks @jenswi-linaro

jenswi-linaro added a commit to jenswi-linaro/optee_os that referenced this issue Nov 23, 2017
@jenswi-linaro
libfdt: fix undefined behaviour in fdt_offset_ptr()
cca4b23 
Upstream commit d0b3ab0a0f46 ("libfdt: Fix undefined behaviour in
fdt_offset_ptr()").

Using pointer arithmetic to generate a pointer outside a known object is,
technically, undefined behaviour in C.  Unfortunately, we were using that
in fdt_offset_ptr() to detect overflows.

To fix this we need to do our bounds / overflow checking on the offsets
before constructing pointers from them.

Acked-by: Jerome Forissier <[email protected]>
Fixes: OP-TEE#1967
Signed-off-by: Jens Wiklander <[email protected]>
jforissier pushed a commit that referenced this issue Nov 23, 2017
@jenswi-linaro @jforissier
libfdt: fix undefined behaviour in fdt_offset_ptr()
a0ffc59 
Upstream commit d0b3ab0a0f46 ("libfdt: Fix undefined behaviour in
fdt_offset_ptr()").

Using pointer arithmetic to generate a pointer outside a known object is,
technically, undefined behaviour in C.  Unfortunately, we were using that
in fdt_offset_ptr() to detect overflows.

To fix this we need to do our bounds / overflow checking on the offsets
before constructing pointers from them.

Acked-by: Jerome Forissier <[email protected]>
Fixes: #1967
Signed-off-by: Jens Wiklander <[email protected]>
takuya-sakata pushed a commit to renesas-rcar/optee_os that referenced this issue May 28, 2018
@jenswi-linaro @takuya-sakata
libfdt: fix undefined behaviour in fdt_offset_ptr()
25c39e1 
Upstream commit d0b3ab0a0f46 ("libfdt: Fix undefined behaviour in
fdt_offset_ptr()").

Using pointer arithmetic to generate a pointer outside a known object is,
technically, undefined behaviour in C.  Unfortunately, we were using that
in fdt_offset_ptr() to detect overflows.

To fix this we need to do our bounds / overflow checking on the offsets
before constructing pointers from them.

Acked-by: Jerome Forissier <[email protected]>
Fixes: OP-TEE/optee_os#1967
Signed-off-by: Jens Wiklander <[email protected]>
jordanrh1 pushed a commit to ms-iot/optee_os that referenced this issue Oct 16, 2018
@jenswi-linaro @sahilnxp
libfdt: fix undefined behaviour in fdt_offset_ptr()
f67cf44 
Upstream commit d0b3ab0a0f46 ("libfdt: Fix undefined behaviour in
fdt_offset_ptr()").

Using pointer arithmetic to generate a pointer outside a known object is,
technically, undefined behaviour in C.  Unfortunately, we were using that
in fdt_offset_ptr() to detect overflows.

To fix this we need to do our bounds / overflow checking on the offsets
before constructing pointers from them.

Acked-by: Jerome Forissier <[email protected]>
Fixes: OP-TEE/optee_os#1967
Signed-off-by: Jens Wiklander <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

libfdt: fix undefined behaviour in fdt_offset_ptr() jenswi-linaro/optee_os
2 participants
@viennadd @jenswi-linaro