Page MenuHomeFreeBSD
Differential D54547

wtap: Remove bogus NULL check in wtap_transmit
Needs ReviewPublic
Actions

Authored by enweiwu on Tue, Jan 6, 12:40 AM.
Tags
Referenced Files
Unknown Object (File)
Sat, Jan 10, 11:51 AM
Unknown Object (File)
Sat, Jan 10, 5:42 AM
Unknown Object (File)
Sat, Jan 10, 5:32 AM
Unknown Object (File)
Fri, Jan 9, 7:58 PM
Unknown Object (File)
Thu, Jan 8, 11:28 PM
Unknown Object (File)
Wed, Jan 7, 4:12 PM
Unknown Object (File)
Wed, Jan 7, 4:00 PM
Unknown Object (File)
Wed, Jan 7, 7:58 AM
View All 13 Files
Subscribers
enweiwu
imp
wireless

Details

Reviewers
adrian
bz
Group Reviewers
wireless
Summary

The node pointer is guaranteed to be non-NULL by the net80211 stack.

The original check was also ineffective as it dereferenced ni->ni_vap before the NULL check.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

enweiwu created this revision.Tue, Jan 6, 12:40 AM
Herald added a subscriber: imp. · View Herald TranscriptTue, Jan 6, 12:40 AM
enweiwu requested review of this revision.Tue, Jan 6, 12:40 AM
bz added a comment.Tue, Jan 6, 12:50 AM

Can you please create diffs with -U9999 if you manually upload them so there is context?

bz requested changes to this revision.Tue, Jan 6, 12:56 AM
bz added inline comments.
sys/dev/wtap/if_wtap.c
594

The assumption that this can happen seems wrong. This should probably be a MPASS(ni != NULL); and we should be done with it.

Ignoring this for a minute if you look at ieee80211_parent_xmitpkt() which calls ic->ic_transmit you'll see that in the error case it does the cleanup (which does depend on the ni being valid) so be careful if you already free the mbuf (likely wrong). This is different to (*ic_raw_xmit) where you'd need to free the mbuf.

This revision now requires changes to proceed.Tue, Jan 6, 12:56 AM
enweiwu marked an inline comment as done.Tue, Jan 6, 8:15 PM
enweiwu added inline comments.
sys/dev/wtap/if_wtap.c
594

So, turns out the NULL check on the node ni is unnecessary in the first place. Let me revise the commit by removing the NULL check on the node ni.

enweiwu updated this revision to Diff 169210.Tue, Jan 6, 8:18 PM
enweiwu marked an inline comment as done.
enweiwu retitled this revision from wtap(4): Fix NULL pointer dereference in wtap_transmit to wtap: Remove bogus NULL check in wtap_transmit.
enweiwu edited the summary of this revision. (Show Details)

Revision Contents
Changeset List

PathSize
sys/
dev/
wtap/
8 lines

Diff 169210

View Options

sys/dev/wtap/if_wtap.c

Loading...