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

netsocket: several dynamic allocation results not checked #14210

Open
pea-pod opened this issue Jan 28, 2021 · 2 comments
Open

netsocket: several dynamic allocation results not checked #14210

pea-pod opened this issue Jan 28, 2021 · 2 comments
Labels
component: connectivity priority: untriaged
Projects
Issue Workflow Issue Severity

Comments

@pea-pod
Copy link
Contributor

pea-pod commented Jan 28, 2021 •
edited

Description of defect

There are several netsocket source files that dynamically allocate without std::nothrow or otherwise checking for allocation failure. Here is the list of files, excluding tests:

Target(s) affected by this defect ?

All with onboard networking

Toolchain(s) (name and version) displaying this defect ?

N/A

What version of Mbed-os are you using (tag or sha) ?

mbed-os-6.7.0

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

N/A

How is this defect reproduced ?

Any allocation failure should result in crashing either in allocation failure or attempting to use an unallocated resource. For the most part, the failure should be handled and returned as an error instead.
@pea-pod
Copy link
Contributor Author

pea-pod commented Jan 28, 2021

I first noticed this while trying to figure out if it would be possible inherit TCPSocket to override accept to use a MemoryPool instead of actual dynamic allocation. I wanted to allocate memory statically and not while running the server if possible.

I realized that the accept method never checks to see if the new call to the protected constructor and could fail. It is possible that I've experienced this while testing out an idea and never realized it because I did not want to get the haruspex diagram out (my hepatomancy license has lapsed).

@ciarmcom
Copy link
Member

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers.
Internal Jira reference: https://jira.arm.com/browse/IOTOSM-3331

@pea-pod pea-pod mentioned this issue Feb 1, 2021
Closed
@chrisswinchatt-arm chrisswinchatt-arm mentioned this issue Apr 1, 2021
Closed
@ciarmcom ciarmcom added this to In Progress in Issue Workflow May 21, 2021
@ciarmcom ciarmcom added this to Untriaged in Issue Severity May 21, 2021
@chrisswinchatt-arm chrisswinchatt-arm mentioned this issue May 26, 2021
Closed
chrisswinchatt-arm added a commit to chrisswinchatt-arm/mbed-os that referenced this issue Jun 7, 2021
@chrisswinchatt-arm
Fix 'netsocket: several dynamic allocation results not checked' (ARMm…
5a92d87 
…bed#14210)

Incorporates PR ARMmbed#14223 + changes required for spellchecker
@chrisswinchatt-arm chrisswinchatt-arm mentioned this issue Jun 7, 2021
Merged
chrisswinchatt-arm added a commit to chrisswinchatt-arm/mbed-os that referenced this issue Jun 7, 2021
@chrisswinchatt-arm
Fix 'netsocket: several dynamic allocation results not checked' (ARMm…
053eb24 
…bed#14210)

Incorporates PR ARMmbed#14223 + changes required for spellchecker
0xc0170 added a commit that referenced this issue Jun 9, 2021
@0xc0170
Merge pull request #14740 from chrisswinchatt-arm/fix-netsocket-dynalloc
0bbc3e2 
Fix 'netsocket: several dynamic allocation results not checked' (#14210)

add_event_listener in NetworkInterface now returns an error if the method fails. Previous attempts to add the event listener would attempt to use an unchecked standard dynamically allocated ns_list_* item.

In other cases, the dynamically allocated items will now be checked, and if unsuccessful, will return after cleaning up any outstanding issues.

TCPSocket::accept will now check that its own internally allocated new TCPSocket call will succeed, and if not, will clean up the stack resources. This should help when memory is low but an incoming connection requests a connection when the TCPSocket is listening.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component: connectivity priority: untriaged
Projects
Issue Severity
Untriaged
Issue Workflow
In Progress
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Handle failed memory allocations in netsockets
3 participants
@0xc0170 @ciarmcom @pea-pod