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

libfdt: fix undefined behaviour in fdt_offset_ptr() #1969

Merged
merged 1 commit into from
Nov 23, 2017

Conversation

jenswi-linaro
Copy link
Contributor

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: #1967
Signed-off-by: Jens Wiklander jens.wiklander@linaro.org

@jforissier
Copy link
Contributor

Acked-by: Jerome Forissier <jerome.forissier@linaro.org>

(small "process" question: do we want A-b/R-b tags for upstream fixes like this one? or is it enough that one core member of the team picks the patch and carries it with his S-o-b?)

@jenswi-linaro
Copy link
Contributor Author

Good question, I was thinking more or less the same. When it's just a cherry-pick like in this case I think it would make sense to preserve the original tags. We could separate them from our tags with an empty line to show that they originate from an earlier process.

@jforissier
Copy link
Contributor

Separating the tags sounds good in general, but sometimes we may want to adjust the commit log to give additional info specific to OP-TEE. It can also happen that the upstream patch doesn't apply cleanly and needs adjustments. So I think it's better to consider it as a new patch, and apply only "our" tags. It is easy enough to look up the original patch, as long as it is properly cited as you did.

Let's also keep the same rules as usual patches, i.e., wait for at least one ack or review (meaning "I agree it is OK to cherry-pick this fix now").

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 <jerome.forissier@linaro.org>
Fixes: OP-TEE#1967
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
@jenswi-linaro
Copy link
Contributor Author

OK
Tag applied

@jforissier jforissier merged commit a0ffc59 into OP-TEE:master Nov 23, 2017
@jenswi-linaro jenswi-linaro deleted the libfdt_fix branch November 23, 2017 10:23
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.

Using Undefined Behavior to detect pointer arithmetic overflow in older version of libfdt
2 participants