ng_parse: disallow negative length for malloc
ClosedPublic
Actions

Authored by emaste on Nov 1 2022, 2:12 PM.

Details

Reviewers

donner
glebius

Commits

rGae4f39464c61: ng_parse: disallow negative length for malloc

Summary

PR:             267334
Reported by:    Robert Morris <rtm@lcs.mit.edu>
Sponsored by:   The FreeBSD Foundation

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

emaste created this revision.Nov 1 2022, 2:12 PM

Herald added subscribers: glebius, melifaro. · View Herald TranscriptNov 1 2022, 2:12 PM

emaste requested review of this revision.Nov 1 2022, 2:12 PM

just return, foff is not yet set

Can we return unsigned value from ng_get_composite_len() instead?

This revision now requires changes to proceed.Nov 2 2022, 6:14 PM

Looking at 267334. IMHO, we should return the error for invalid message as close as possible to the userland. Don't let the invalid data travel this down.

Of course we should also check for num nonnegative and too large; there's probably more to be covered here too.

Can we return unsigned value from ng_get_composite_len() instead?

We should, but I think there are ABI considerations to take into account and we really need someone who understands netgraph well.

I'm going to commit this as an interim improvement. We can revisit ng_get_composite_len's return type and error handling later on.

This revision was not accepted when it landed; it landed in state Needs Revision.Thu, Nov 21, 8:56 PM

Closed by commit rGae4f39464c61: ng_parse: disallow negative length for malloc (authored by emaste). · Explain Why

This revision was automatically updated to reflect the committed changes.

emaste added a commit: rGae4f39464c61: ng_parse: disallow negative length for malloc.

Herald added a subscriber: imp. · View Herald TranscriptThu, Nov 21, 8:56 PM

Revision Contents
Changeset List

Path

Size

sys/

netgraph/

ng_parse.c

2 lines

Diff 146813

View Options

ng_parse: disallow negative length for mallocClosedPublicActions