Page MenuHomeFreeBSD

bluetooth socket sysinit: correct memset initialization
Needs ReviewPublic

Authored by rlibby on Sun, Jun 23, 4:38 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Jun 27, 3:31 PM
Unknown Object (File)
Wed, Jun 26, 4:59 PM

Details

Reviewers
markj
takawata
Summary

gcc -Wmemset-elt-size diagnosed this. The code was only initializing
the first 1/sizeof(long) bytes. On 64-bit systems, this would mean only
events up to 0x20 were initialized.

This effectively reverses the security policy for some events with
higher ids, now permitting them on unprivileged sockets. Two that are
defined are NG_HCI_EVENT_LE (0x3e) and NG_HCI_EVENT_BT_LOGO (0xfe).


I am not familiar with this code, am not planning to test bluetooth, and
am not in any rush to commit this, I just saw that this warning appeared
to be a genuine bug. I do not know what the appropriate security policy
actually is. Please feel free to take this patch over if desired.

Test Plan

make buildkernel

Diff Detail

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

Event Timeline

rlibby edited the test plan for this revision. (Show Details)
rlibby added reviewers: markj, takawata.

Given that the next 70 lines are mostly manually setting bits in the mask, I'd err on the side of entirely removing this initialization.

Given that the next 70 lines are mostly manually setting bits in the mask, I'd err on the side of entirely removing this initialization.

It's actually only the next three operations that set bits in events. f is reassigned after that. So just removing the memset would be a change to permit no events on unprivileged sockets.

I do think that using default deny and explicit allows may be safer in general, though I don't know how much evolution of events there has been.

Anyway, I'm not invested enough in this code to go research events and implement something cleaner. Would it be better just to file a bugzilla bug for this?