Page MenuHomeFreeBSD

wtap(4): Fix bug in wtap_node_write() and wtap_vap_create()
ClosedPublic

Authored by enweiwu on Jul 8 2022, 6:35 AM.

Details

Summary

Planned commit message:

[wtap] Fix bug in wtap_node_write() and wtap_vap_create()

bug1:
    Originally, wtap_node_write() gets the wrong softc by iterating V_inet and get the ifp by string comparison, then gets softc by ifp->if_softc. However, ifp->if_softc will not points to the correct softc owned by ieee80211com, and thus cause kernel panic. Fix it by assigning softc to cdev's si_drv1 in wtap_vap_create(), and gets softc via dev->si_drv1 in wtap_node_write().

bug2:
    The cdev created by wtap_vap_create() use the name of ieee80211com rather than the vap's name. It will cause the second vap based on the same ieee80211com as first vap fails to create device node because the device node is already exists. Fix it by assigning vap->iv_ifp->if_xname to cdev's name.

This diff depends on D35751.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

enweiwu created this revision.
enweiwu retitled this revision from Fix bug in wtap_node_write() and wtap_vap_create() to [wtap] Fix bug in wtap_node_write() and wtap_vap_create().Jul 8 2022, 6:55 AM
enweiwu edited the summary of this revision. (Show Details)
enweiwu retitled this revision from [wtap] Fix bug in wtap_node_write() and wtap_vap_create() to [wtap(4)] Fix bug in wtap_node_write() and wtap_vap_create().
enweiwu edited the summary of this revision. (Show Details)
enweiwu retitled this revision from [wtap(4)] Fix bug in wtap_node_write() and wtap_vap_create() to wtap(4): Fix bug in wtap_node_write() and wtap_vap_create().
This revision is now accepted and ready to land.Jul 8 2022, 3:40 PM
bz added inline comments.
sys/dev/wtap/if_wtap.c
108

Do you still need to hold the epoch for wtap_inject here?

111

All the printfs in that code should eventually be changed into some kind of debug logging. But not in this change.

@lwhsu do you want me to push this to main or do you want to?

In D35752#813636, @bz wrote:

@lwhsu do you want me to push this to main or do you want to?

Yes this is indeed one thing I want to check with you, and it would be very nice if you can help this.

I will work with @enweiwu about changing hard-coded printf with debug macros.

Can you please update the review after doing a git rebase? It doesn't apply to main.

In D35752#816864, @bz wrote:

Can you please update the review after doing a git rebase? It doesn't apply to main.

I've checked that it can be applied to the current HEAD of main branch ( 2c4276aaa2d03217b9c1797196864bbbe4f2d8ea ). Is there anything we missed?

In D35752#816864, @bz wrote:

Can you please update the review after doing a git rebase? It doesn't apply to main.

I've checked that it can be applied to the current HEAD of main branch ( 2c4276aaa2d03217b9c1797196864bbbe4f2d8ea ). Is there anything we missed?

Hmm..

I just tried again:
% arc patch D35752
gives errors and rejects the patch. It seems it also tries to touch more files. Doh! There is an open stack and I didn't put in --skip-dependencies.
Sorry about that. My fault. I'll go and put this in and then we can try to rebase D35841. Really sorry.