Page MenuHomeFreeBSD

Add bounds checking to the tws(4) passthrough ioctl handler.
ClosedPublic

Authored by markj on Dec 12 2018, 11:40 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Oct 15, 1:11 AM
Unknown Object (File)
Wed, Oct 15, 1:10 AM
Unknown Object (File)
Wed, Oct 15, 1:10 AM
Unknown Object (File)
Tue, Oct 14, 4:24 PM
Unknown Object (File)
Tue, Oct 14, 1:54 AM
Unknown Object (File)
Wed, Oct 8, 7:30 AM
Unknown Object (File)
Tue, Oct 7, 2:38 PM
Unknown Object (File)
Tue, Sep 30, 7:38 PM
Subscribers

Details

Summary

tws_passthru() does a copyin of a user-specified request without
validating the length. This can only be exploited by root, however.

Test Plan

None. I have no way of testing this.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj added reviewers: delphij, emaste.
delphij added a subscriber: jpaetzel.

LGTM (the unlocked use of sc->ioctl_data_mem looks worrisome to me, but the proposed change won't worsen the situation). Do you have a chance to test this on real hardware? (@jpaetzel do you know someone who may be able to help with that?).

This revision is now accepted and ready to land.Dec 13 2018, 12:53 AM

LGTM (the unlocked use of sc->ioctl_data_mem looks worrisome to me, but the proposed change won't worsen the situation).

I was wondering about that too. AFAIK there is no mechanism at the upper layers to serialize calls to the ioctl handler, so two threads can race here.

Do you have a chance to test this on real hardware? (@jpaetzel do you know someone who may be able to help with that?).

I don't own any hardware driven by tws. Any help on that front would be much appreciated.

LGTM (the unlocked use of sc->ioctl_data_mem looks worrisome to me, but the proposed change won't worsen the situation). Do you have a chance to test this on real hardware? (@jpaetzel do you know someone who may be able to help with that?).

My last 9750 died a while ago. I'll ping Austin @ ix to see if he can rig up a system for us to test with.

LGTM (the unlocked use of sc->ioctl_data_mem looks worrisome to me, but the proposed change won't worsen the situation). Do you have a chance to test this on real hardware? (@jpaetzel do you know someone who may be able to help with that?).

My last 9750 died a while ago. I'll ping Austin @ ix to see if he can rig up a system for us to test with.

Ping?

LGTM (the unlocked use of sc->ioctl_data_mem looks worrisome to me, but the proposed change won't worsen the situation). Do you have a chance to test this on real hardware? (@jpaetzel do you know someone who may be able to help with that?).

My last 9750 died a while ago. I'll ping Austin @ ix to see if he can rig up a system for us to test with.

Ping?

I haven't been deemed worthy of a reply, so I guess that's a no.

LGTM (the unlocked use of sc->ioctl_data_mem looks worrisome to me, but the proposed change won't worsen the situation). Do you have a chance to test this on real hardware? (@jpaetzel do you know someone who may be able to help with that?).

My last 9750 died a while ago. I'll ping Austin @ ix to see if he can rig up a system for us to test with.

Ping?

I haven't been deemed worthy of a reply, so I guess that's a no.

Thanks, let's land the change then.

LGTM (the unlocked use of sc->ioctl_data_mem looks worrisome to me, but the proposed change won't worsen the situation). Do you have a chance to test this on real hardware? (@jpaetzel do you know someone who may be able to help with that?).

My last 9750 died a while ago. I'll ping Austin @ ix to see if he can rig up a system for us to test with.

Ping?

I haven't been deemed worthy of a reply, so I guess that's a no.

Thanks, let's land the change then.

Yeah, it's simple enough that I'm not too worried.

This revision was automatically updated to reflect the committed changes.