Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)

[Tejun Heo <tj@kernel.org>]

Hello,

On Sat, Nov 03, 2012 at 12:19:05AM +0000, Alan Cox wrote:
> > Hmmm?  You know which commands you're allowing.  You can definitely
> > filter those commands for their ranges.  What ioctls?
> 
> How do you know what the rules are in kernel. If I'm locking you to fixed
> mappings you have no idea in kernel what my user policy model is.

So, my first response was whether you mean to add arbitrary range
filtering for standard read/writes too.  If you're not gonna do that
and use the existing partition based access model, it's natural to
apply the same partition ranges to the allowed SG_IO commands, right?
There's no new access model which should be configured here.  It just
applied the same block device access model to SG_IO commands too.

> > > So that translates to me as "There is a good reason, but if your drive is
> > > one of the awkward ones then f**k you go use root". Again policy in the
> > > kernel just creates inflexibility and is the wrong place for it.
> > 
> > Yes, pretty much. 
> 
> Unfortunate and guaranteed to end up with problems not getting fixed
> again - or having to redo the work a second time later on. Plus this is
> but one example and you are blocking all the ones that haven't been
> considered.

But we should't add features for the ones which haven't been
considered.  Unless you can actually justify with actual use cases,
it's just hand waving.

> > > If you are doing virtual machines it is far from marginal.
> > 
> > Yeah, I agree VMs are the only one which isn't marginal, but then
> > again, VMs can work reasonably well with far simpler mechanism.
> 
> I'm dying to see your "simpler mechanism" - I bet by the time you've
> written the code it isn't simpler than calling the existing BPF logic.
> Your kernel already has all the BPF stuff in it unless you are building
> with no networking support so its essentially free.

The suggested mechanism is just having a switch to allow all SG_IO
commands and pass it to the hypervisor.  There's no filtering in
kernel at all.

> We have filtering because it is necessary. All you are doing is putting
> off the inevitable by adding more kernel hack "one true kernel enforced
> religion" policy and putting off the inevitable while adding APIs you'll
> then have to maintain until the job is done right.
> 
> Ultimately policy has to be user space driven, adding more temporary
> hacks is just a waste.

Exactly, let's provide a turn-off switch for in-kernel filtering and
let userland drive any appropriate access policy on it.  Let's please
stay away from doing deep packet inspecting on SG_IO commands.

I don't think we're disagreeing on the principle.  It's just that
we're drawing different where the line lies between mechanisms and
policies.

Thanks.

-- 
tejun