NAK: [bionic][PATCH 00/10] Add drivers for RTL8821C WiFi and BT

[Anthony Wong]

Hi Seth,

Kai-heng took another stab at the 8821CE wifi driver by removing lots
of unnecessary codes to make it more secure (it's still quite big
TBH), in the hope that our projects can use this device directly with
our kernels. He has sent a pull request in another email, would you
mind giving it a review?

Thanks,
Anthony

On Wed, Mar 28, 2018 at 07:58:54AM -0500, Seth Forshee wrote:
> On Wed, Mar 28, 2018 at 04:12:09PM +0800, Jesse Sung wrote:
> > Hi Seth,
> > 
> > On Tue, Mar 27, 2018 at 10:10 PM, Seth Forshee
> > <seth.forshee at canonical.com> wrote:
> > > On Sat, Mar 24, 2018 at 12:46:57AM +0800, Wen-chien Jesse Sung wrote:
> > >> BugLink: https://launchpad.net/bugs/1740231
> > >> BugLink: https://launchpad.net/bugs/1742613
> > >>
> > >> These are based on the drivers provided by Realtek for 4.13.
> > >>
> > >> Since btusb will bind to this bluetooth device, it must be blacklisted
> > >> in btusb to make sure that the correct driver is used. Also the table in
> > >> ubuntu/rtl8821c-bt is modified so that the driver works only for this
> > >> device only.
> > >>
> > >> Risk should be low since both drivers have no impact for systems without
> > >> these devices.
> > >
> > > This is a driver with almost 600,000 lines of code, submitted at the
> > > last minute without any explanation of why we should be adding it to our
> > > kernel. Please explain why this is needed.
> > 
> > This device was enabled on some HP laptops. Right now we're supporting
> > it with linux-oem 4.13 in Xenial. In order to keep this device working after
> > upcoming linux-oem upgrade/migration, the best way is to integrate this into
> > bionic kernel.
> 
> Okay, I do agree that we want to allow users to upgrade without breaking
> their hw support. However this driver is ridiculously large. I have no
> idea how anyone could possibly end up with a driver that has nearly
> 600,000 lines of code. This just doesn't belong in the distro kernel,
> imo.
> 
> > > Some of my objections/questions from just a quick skim:
> > >
> > >  - Being built with our kernel, the driver will be signed and thus
> > >    eligible to be loaded during secure boot. There's no chance that we
> > >    can review it to ensure that it doesn't expose any unsafe interfaces
> > >    to userspace. Have you done such a review?
> > 
> > For platforms w/o this device, if one can load a module to expose something,
> > he can always load anything harmful directly anyway. But for platforms w/ the
> > device, I'd admit that I have the same doubt as you.
> 
> We carry a set of patches for "lockdown" under secure boot. Lockdown
> means that only signed modules can be loaded and a variety of
> potentially insecure interfaces to userspace are disabled. If a user
> wants to load an unsigned module they first have to disable secure boot
> validation.
> 
> Shipping a signed module with our kernel means that we believe that
> either the module doesn't expose any insecure interfaces to userspace or
> else those interfaces are restricted when lockdown is enforced. I doubt
> this driver has been evaluated with this in mind.
> 
> > >  - Why does this need to hook into our debian build scripts rather than
> > >    being built like a normal module?
> > 
> > I tried to keep the modification to the driver as minimal as possible, in case
> > that there's an update (a new tarball) from Realtek. But yes, I can modify
> > the Makefile to make it look like a normal module if it's preferred.
> > 
> > >
> > >  - You blacklist usb device 0bda:b00a for all architectures but only
> > >    build the replacement driver for x86. Is there any chance someone
> > >    might want to use the device with another architecture? If so you'll
> > >    be breaking them.
> > 
> > Since the btusb doesn't work with this device, this may not be a problem.
> > 
> > Thanks,
> > Jesse
> > 
> > >
> > >  - Presumably we'll have to keep maintaining this huge pile of code for
> > >    years into the future, up to and including the next LTS I expect.
> > >
> > > Thanks,
> > > Seth
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

-- 
Acked-by: Anthony Wong <anthony.wong at canonical.com>
-- 
Anthony Wong <anthony.wong at canonical.com>
Engineering Manager, Hardware Enablement, Kernel Team
Canonical Ltd.      www.canonical.com