Page MenuHomeFreeBSD

if_vxlan(4): fix panic by validating unused drvspec values
ClosedPublic

Authored by pouria on Thu, Jul 30, 1:19 PM.
Tags
None
Referenced Files
F164719838: D58552.id183108.diff
Mon, Aug 3, 8:28 AM
F164672364: D58552.id183104.diff
Mon, Aug 3, 2:37 AM
F164658633: D58552.id183106.diff
Mon, Aug 3, 12:55 AM
F164646889: D58552.id183104.diff
Sun, Aug 2, 9:32 PM
F164646886: D58552.id183106.diff
Sun, Aug 2, 9:32 PM
F164646885: D58552.diff
Sun, Aug 2, 9:32 PM
Unknown Object (File)
Sun, Aug 2, 6:35 AM
Unknown Object (File)
Sun, Aug 2, 5:31 AM
Subscribers

Details

Summary

Add validation for unused parameter values in the gap between
VXLAN_PARAM_WITH_LOCAL_ADDR4 and VXLAN_PARAM_WITH_LOCAL_ADDR6 to prevent
panics.

PR: 297151
Reported by: Robert Morris <rtm@lcs.mit.edu>
MFC after: 3 days

Test Plan

See PR297151

Diff Detail

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

Event Timeline

I also can fix this by adding extra validation check in vxlan_ioctl_drvspec() to avoid renumbering.
But, this driver only uses ioctl and I've plans to add the support for netlink + vnet too.
I'm fine if anyone want me to add the extra check instead.

MFC after: 3 days

This breaks the userspace ABI. Old ifconfig binaries won't work properly with a new kernel after this change.

This breaks the userspace ABI. Old ifconfig binaries won't work properly with a new kernel after this change.

I understand, that's why I said I can do it the other way.
I want to make sure it's needed, because users usually update their userland + kernel together.
So should I do it the other way?

This breaks the userspace ABI. Old ifconfig binaries won't work properly with a new kernel after this change.

I understand, that's why I said I can do it the other way.
I want to make sure it's needed, because users usually update their userland + kernel together.
So should I do it the other way?

Yes, in general FreeBSD tries quite hard to avoid breaking the kernel<->userspace ABI. There are exceptions, but if it's easy to avoid this breakage, then we should.

It's very useful to be able to upgrade the kernel without upgrading userspace, and one might want an ifconfig binary from a jail to continue working even if the host is upgraded.

pouria retitled this revision from if_vxlan(4): Fix panic by renumbering ifvxlanparam to if_vxlan(4): fix panic by validating unused drvspec values.Thu, Jul 30, 1:45 PM
pouria edited the summary of this revision. (Show Details)
pouria edited the test plan for this revision. (Show Details)
sys/net/if_vxlan.h
115 ↗(On Diff #183104)

It would be simpler to just test vc->vxlc_func == NULL and return EINVAL if so. Then you don't need to keep track of the holes in the command space.

This revision is now accepted and ready to land.Thu, Jul 30, 2:08 PM