Page MenuHomeFreeBSD
Differential D60184

netinet6: Fix the OSIOCAIFADDR_IN6 handler
AcceptedPublic
Actions

Authored by markj on Wed, Sep 30, 8:25 PM.
Tags
None
Referenced Files
F174249959: D60184.id188241.diff
Thu, Oct 1, 5:40 PM
F174249708: D60184.diff
Thu, Oct 1, 5:37 PM
F174237276: D60184.id.diff
Thu, Oct 1, 3:01 PM
F174180380: D60184.id188241.diff
Thu, Oct 1, 4:43 AM
F174168836: D60184.diff
Thu, Oct 1, 2:42 AM
Unknown Object (File)
Wed, Sep 30, 11:10 PM
Unknown Object (File)
Wed, Sep 30, 11:04 PM
Subscribers
ae
glebius
imp
melifaro
pouria

Details

Reviewers
pouria
Group Reviewers
network
Summary

We cannot simply treat a pointer to struct oin6_aliasreq as a pointer to
struct in6_aliasreq: the latter is larger. In particular, the store to
ifra->ifra_vhid is out of bounds.

Since OSIOCAIFADDR_IN6 does not need to copy data back out, it can
simply use a struct in6_ifaliasreq on the stack.

Reported by: Andrew <xxx.sys@protonmail.com>

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 77536
Build 74419: arc lint + arc unit

Event Timeline

markj created this revision.Wed, Sep 30, 8:25 PM
Herald added subscribers: pouria, glebius, melifaro and 2 others. · View Herald TranscriptWed, Sep 30, 8:25 PM
markj requested review of this revision.Wed, Sep 30, 8:25 PM
Harbormaster completed remote builds in B77536: Diff 188241.Wed, Sep 30, 8:25 PM
pouria accepted this revision.Thu, Oct 1, 2:28 PM
pouria added inline comments.
sys/netinet6/in6.c
274–276

Why keep the compatibility at all?

This revision is now accepted and ready to land.Thu, Oct 1, 2:28 PM
markj marked an inline comment as done.Thu, Oct 1, 2:40 PM
markj added inline comments.
sys/netinet6/in6.c
274–276

It's useful for running old FreeBSD versions in a jail, I suppose.

pouria added inline comments.Thu, Oct 1, 2:47 PM
sys/netinet6/in6.c
274–276

It's useful for running old FreeBSD versions in a jail, I suppose.

Pre-10? (pre 2014) + on IPv6? (adaptation rate on that year) + inside a jail? (limited to vnet)
IMHO, not worth extra memset+memcpy.

markj marked an inline comment as done.Thu, Oct 1, 2:51 PM
markj added inline comments.
sys/netinet6/in6.c
274–276

Maybe it's not worth keeping, but I will let someone else handle that question. Even if we remove it in main, this patch should still be MFCed to stable branches, so I would like to fix this properly.

IMHO, not worth extra memset+memcpy.

I'm not sure what you mean. That cost is only incurred if you use the compat ioctl. There is no overhead otherwise.

pouria added inline comments.Thu, Oct 1, 3:01 PM
sys/netinet6/in6.c
274–276

Even if we remove it in main, this patch should still be MFCed to stable branches, so I would like to fix this properly.

Make sense!

I'm not sure what you mean. That cost is only incurred if you use the compat ioctl. There is no overhead otherwise.

I understand, infact it might directly skip all of this up to ifp->if_ioctl, since I don't see any switch case that it could match with it at all.
What I mean is the function is already long enough.
Of course, it make sense for MFCing.

Revision Contents
Changeset List

PathSize
sys/
netinet6/
6 lines

Diff 188241

View Options

sys/netinet6/in6.c

Loading...