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

Consider limiting size of read/write/seek to INT32_MAX #657

Open
jphickey opened this issue Nov 17, 2020 · 1 comment
Open

Consider limiting size of read/write/seek to INT32_MAX #657

jphickey opened this issue Nov 17, 2020 · 1 comment
Labels

Comments

@jphickey
Copy link
Contributor

Is your feature request related to a problem? Please describe.
The OSAL API returns results as int32, and this includes the size of data read/written from operations like OS_read and OS_write. However it is possible to read/write a larger buffer than what can be expressed as int32. If this overflow happens then the result is likely to become negative and be interpreted as an error.

Describe the solution you'd like
OS_read and OS_write should probably cap the amount they will transfer in a single call to be INT32_MAX. This should in turn limit the size of the result that would need to be returned to the caller.

OS_lseek() returns the file offset, so this probably doesn't work with files bigger than 2GB.

Describe alternatives you've considered
Use a larger data type e.g. int64 as return, but this is potentially slow on 32 bit CPUs where 64 bit values may need to be emulated by the C library.

Additional context
If size_t (buffer size parameter) is 64 bits and the return value is 31 bits (usable) then there is a large set of potential values which are not representable.

However - this problem has existed even when the input size was uint32 rather than size_t ... because anything bigger than INT32_MAX is a problem - so this isn't new, its just potentially more of a concern with large files./file systems and 64 bit platforms.

Read/Write actions should always be allowed (per API) to transfer fewer bytes than the request was for - app should retry with the remainder. So capping at INT32_MAX should not be a problem - no app should expect an extremely large transfer like that to happen in one go.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

@skliper skliper added the bug label Dec 1, 2020
@skliper
Copy link
Contributor

skliper commented Jan 4, 2021

I'm in favor of it. Quick fix? If so I'd like to resolve for Caelum.

jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Add missing UT_Stub_RegisterContext to CFE_EVS_SendEvent and CFE_ES_WriteToSysLog.
This will create the correct register buffer size.
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
…ontext

Fix nasa#657, Add UT_Stub_RegisterContext to CFE_EVS_SendEvent and CFE_ES_WriteToSysLog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants