Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
On Mon, Nov 05, 2012 at 08:17:54PM +, Alan Cox wrote: > IMHO that's the best option for now - if it fixes the specific case of > concern right now. It's a trivial interface, it's trivial security (need > to check CAP_SYS_RAWIO to flip) and it means that the job can be done > properly when there is consensus without a whole extra legacy API stuck > behind. Oh yeah, fully agreed. We could just set the sysfs file only writeable by root (basically just setting perm to 0644). Would the CAP_SYS_RAWIO distinction matter? Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
On Mon, 5 Nov 2012 12:09:55 -0800 Tejun Heo wrote: > Hey, Alan. > > On Mon, Nov 05, 2012 at 08:12:08PM +, Alan Cox wrote: > > There are two sensible choices here IMHO > > > > - The simple sysctl > > > > - Doing the job right > > > > A half way solution such as that you are proposing seems to me to achieve > > nothing other than guaranteeing we'll have another pile of useless crap > > to maintain forever as ABI. > > I'm all for simple. Just throw in a sysfs binary switch to allow all > SG_IO for a given block device for users w/ write access. I'd be > completely happy with that. IMHO that's the best option for now - if it fixes the specific case of concern right now. It's a trivial interface, it's trivial security (need to check CAP_SYS_RAWIO to flip) and it means that the job can be done properly when there is consensus without a whole extra legacy API stuck behind. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hey, Alan. On Mon, Nov 05, 2012 at 08:12:08PM +, Alan Cox wrote: > There are two sensible choices here IMHO > > - The simple sysctl > > - Doing the job right > > A half way solution such as that you are proposing seems to me to achieve > nothing other than guaranteeing we'll have another pile of useless crap > to maintain forever as ABI. I'm all for simple. Just throw in a sysfs binary switch to allow all SG_IO for a given block device for users w/ write access. I'd be completely happy with that. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
> To me, it feels like you guys are pushing a feature without strong > enough use case. So, I'm still pretty strongly against it. A feature people have been asking for repeatedly for ten years. Go scan the archive. I'm also pushing for a *generic* do it once solution so we never have to have the debate again regardless of what media pops up in future. There are two sensible choices here IMHO - The simple sysctl - Doing the job right A half way solution such as that you are proposing seems to me to achieve nothing other than guaranteeing we'll have another pile of useless crap to maintain forever as ABI. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hey, Paolo. On Sat, Nov 03, 2012 at 02:20:14PM +0100, Paolo Bonzini wrote: > As to implementing the ioctl, it's all but trivial. For one thing, you > have to make the block device ioctl op take a "struct file". I have > been asking Al Viro about it for 6 months and I haven't got any answer yet. I don't think that really matters. If necessary, just change it. > Second, getting a security-sensitive ioctl right is hard, as you > demonstrated yourself in this thread by proposing a gapingly insecure > approach. Adding a little bit of customization to the current solution > may be but a local optimum, but you cannot really get it wrong. Eh? Sure, I was in "who cares about SG_IO?" area a bit but that has nothing to do with the interface being an ioctl. It's a binary switch ioctl, it's not too difficult to get right. > Because _this_ is the simplest possible. I guess we'll have to agree to disagree. This is way more complex. You need to review what's going on with the userland tools and inspect weird sysfs files to actually know what the system policy is. > I proposed a way to implement the ultimately flexible solution (BPF) and > you shot it down because it was too complex. Alan is showing you with > multiple examples of why the flexibility would be useful (perhaps nobody > would use it, but the use cases _are_ there), and you are mostly > ignoring them. The only valid use case was using proprietary commands during CD burning. At this point, that's a really really minor use case IMHO. I think adding full BPF filtering on SG_IO for that is rather silly. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hello, Alan. On Sat, Nov 03, 2012 at 02:50:52PM +, Alan Cox wrote: > > I proposed a way to implement the ultimately flexible solution (BPF) and > > you shot it down because it was too complex. Alan is showing you with > > multiple examples of why the flexibility would be useful (perhaps nobody > > would use it, but the use cases _are_ there), and you are mostly > > ignoring them. > > My feeling too - It feels to me like Tejun is trying to railroad a broken > non-solution into the system without regards for anyone else and by > simply dismissing any other input. The only other use case brought up is allowing use of vendor-specific commands while burning CDs. Given that the usual burning has been working well enough for years now, I don't really think that's a strong enough reason to add full BPF filtering to SG_IO. It's just highly unusual thing to do and there isn't strong enough use case for it. To me, it feels like you guys are pushing a feature without strong enough use case. So, I'm still pretty strongly against it. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Il 03/11/2012 15:50, Alan Cox ha scritto: >>> It's not really about the lines of code. It adds a new userland >>> visible interface. As for the "long" list of commands, it depends on >>> how you write it but even if it's textually long it's still very >>> simple in terms of actual complexity. >> >> Sure, but its place is not the kernel. >> >> As to implementing the ioctl, it's all but trivial. For one thing, you >> have to make the block device ioctl op take a "struct file". I have >> been asking Al Viro about it for 6 months and I haven't got any answer yet. > > Just do it - if Al cared he'd have replied about it. Yeah, so far I chickened out because---for the usecases I know, at least---a sysfs knob would be simpler to use. >> I proposed a way to implement the ultimately flexible solution (BPF) and >> you shot it down because it was too complex. Alan is showing you with >> multiple examples of why the flexibility would be useful (perhaps nobody >> would use it, but the use cases _are_ there), and you are mostly >> ignoring them. > > My feeling too - It feels to me like Tejun is trying to railroad a broken > non-solution into the system Thanks for spelling it out. :) > without regards for anyone else and by > simply dismissing any other input. My main worry about the BPF implementation is that I need to do CDB inspection, which is similar to packet inspection but doesn't have an sk_buff. The seccomp guys worked around it by using an ANC opcode to access syscall arguments, but here I really need LD. This means for example that it wouldn't be so easy to use the JIT. These are the usecases that really require BPF: > - giving people access to parts of disks This is quite obvious, but I'm not sure why you would that (other than just because). It feels a bit strange to give access to part of a disk with "absolute" LBAs rather than relative. For people needing access to partitions of LVs, what we really need is better access to block device features. We just got BLKZEROOUT, but access to block devices via fallocate() or lseek(SEEK_HOLE/SEEK_DATA) would be a more interesting and useful thing to do. > - giving end users minimal access to things like SMART but only on drives > where it is safe I'm actually in agreement with Tejun that this is handled more than decently by udisks. And the reason this needs BPF, is because the ATA command set is incredibly complicated. If there were request, SMART access could be moved to the kernel/libata and use sysfs+uevents. This is similar to how media change is in the kernel, and SCSI disks allow limited access to mode pages via sysfs. These uses instead are fine with a bitmap: > - giving a user a SCSI disk or partition to play with but preventing them > issuing weird ass commands that can disrupt other devices in the fabric > (like drive to drive transfers, some kinds of resets, management > commands) > - allowing access to specific vendor specific commands on certain > non-standard CD and DVD drives so they can be used for burning but you > can't trash them > - minimising the ability of a VM to do damage if compromised while > maximising its raw access to a device (I'm obviously trying to champion my own patches, but I'll try to be open minded about it). :) Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Il 03/11/2012 15:50, Alan Cox ha scritto: It's not really about the lines of code. It adds a new userland visible interface. As for the long list of commands, it depends on how you write it but even if it's textually long it's still very simple in terms of actual complexity. Sure, but its place is not the kernel. As to implementing the ioctl, it's all but trivial. For one thing, you have to make the block device ioctl op take a struct file. I have been asking Al Viro about it for 6 months and I haven't got any answer yet. Just do it - if Al cared he'd have replied about it. Yeah, so far I chickened out because---for the usecases I know, at least---a sysfs knob would be simpler to use. I proposed a way to implement the ultimately flexible solution (BPF) and you shot it down because it was too complex. Alan is showing you with multiple examples of why the flexibility would be useful (perhaps nobody would use it, but the use cases _are_ there), and you are mostly ignoring them. My feeling too - It feels to me like Tejun is trying to railroad a broken non-solution into the system Thanks for spelling it out. :) without regards for anyone else and by simply dismissing any other input. My main worry about the BPF implementation is that I need to do CDB inspection, which is similar to packet inspection but doesn't have an sk_buff. The seccomp guys worked around it by using an ANC opcode to access syscall arguments, but here I really need LD. This means for example that it wouldn't be so easy to use the JIT. These are the usecases that really require BPF: - giving people access to parts of disks This is quite obvious, but I'm not sure why you would that (other than just because). It feels a bit strange to give access to part of a disk with absolute LBAs rather than relative. For people needing access to partitions of LVs, what we really need is better access to block device features. We just got BLKZEROOUT, but access to block devices via fallocate() or lseek(SEEK_HOLE/SEEK_DATA) would be a more interesting and useful thing to do. - giving end users minimal access to things like SMART but only on drives where it is safe I'm actually in agreement with Tejun that this is handled more than decently by udisks. And the reason this needs BPF, is because the ATA command set is incredibly complicated. If there were request, SMART access could be moved to the kernel/libata and use sysfs+uevents. This is similar to how media change is in the kernel, and SCSI disks allow limited access to mode pages via sysfs. These uses instead are fine with a bitmap: - giving a user a SCSI disk or partition to play with but preventing them issuing weird ass commands that can disrupt other devices in the fabric (like drive to drive transfers, some kinds of resets, management commands) - allowing access to specific vendor specific commands on certain non-standard CD and DVD drives so they can be used for burning but you can't trash them - minimising the ability of a VM to do damage if compromised while maximising its raw access to a device (I'm obviously trying to champion my own patches, but I'll try to be open minded about it). :) Paolo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hello, Alan. On Sat, Nov 03, 2012 at 02:50:52PM +, Alan Cox wrote: I proposed a way to implement the ultimately flexible solution (BPF) and you shot it down because it was too complex. Alan is showing you with multiple examples of why the flexibility would be useful (perhaps nobody would use it, but the use cases _are_ there), and you are mostly ignoring them. My feeling too - It feels to me like Tejun is trying to railroad a broken non-solution into the system without regards for anyone else and by simply dismissing any other input. The only other use case brought up is allowing use of vendor-specific commands while burning CDs. Given that the usual burning has been working well enough for years now, I don't really think that's a strong enough reason to add full BPF filtering to SG_IO. It's just highly unusual thing to do and there isn't strong enough use case for it. To me, it feels like you guys are pushing a feature without strong enough use case. So, I'm still pretty strongly against it. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hey, Paolo. On Sat, Nov 03, 2012 at 02:20:14PM +0100, Paolo Bonzini wrote: As to implementing the ioctl, it's all but trivial. For one thing, you have to make the block device ioctl op take a struct file. I have been asking Al Viro about it for 6 months and I haven't got any answer yet. I don't think that really matters. If necessary, just change it. Second, getting a security-sensitive ioctl right is hard, as you demonstrated yourself in this thread by proposing a gapingly insecure approach. Adding a little bit of customization to the current solution may be but a local optimum, but you cannot really get it wrong. Eh? Sure, I was in who cares about SG_IO? area a bit but that has nothing to do with the interface being an ioctl. It's a binary switch ioctl, it's not too difficult to get right. Because _this_ is the simplest possible. I guess we'll have to agree to disagree. This is way more complex. You need to review what's going on with the userland tools and inspect weird sysfs files to actually know what the system policy is. I proposed a way to implement the ultimately flexible solution (BPF) and you shot it down because it was too complex. Alan is showing you with multiple examples of why the flexibility would be useful (perhaps nobody would use it, but the use cases _are_ there), and you are mostly ignoring them. The only valid use case was using proprietary commands during CD burning. At this point, that's a really really minor use case IMHO. I think adding full BPF filtering on SG_IO for that is rather silly. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
To me, it feels like you guys are pushing a feature without strong enough use case. So, I'm still pretty strongly against it. A feature people have been asking for repeatedly for ten years. Go scan the archive. I'm also pushing for a *generic* do it once solution so we never have to have the debate again regardless of what media pops up in future. There are two sensible choices here IMHO - The simple sysctl - Doing the job right A half way solution such as that you are proposing seems to me to achieve nothing other than guaranteeing we'll have another pile of useless crap to maintain forever as ABI. Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hey, Alan. On Mon, Nov 05, 2012 at 08:12:08PM +, Alan Cox wrote: There are two sensible choices here IMHO - The simple sysctl - Doing the job right A half way solution such as that you are proposing seems to me to achieve nothing other than guaranteeing we'll have another pile of useless crap to maintain forever as ABI. I'm all for simple. Just throw in a sysfs binary switch to allow all SG_IO for a given block device for users w/ write access. I'd be completely happy with that. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
On Mon, 5 Nov 2012 12:09:55 -0800 Tejun Heo t...@kernel.org wrote: Hey, Alan. On Mon, Nov 05, 2012 at 08:12:08PM +, Alan Cox wrote: There are two sensible choices here IMHO - The simple sysctl - Doing the job right A half way solution such as that you are proposing seems to me to achieve nothing other than guaranteeing we'll have another pile of useless crap to maintain forever as ABI. I'm all for simple. Just throw in a sysfs binary switch to allow all SG_IO for a given block device for users w/ write access. I'd be completely happy with that. IMHO that's the best option for now - if it fixes the specific case of concern right now. It's a trivial interface, it's trivial security (need to check CAP_SYS_RAWIO to flip) and it means that the job can be done properly when there is consensus without a whole extra legacy API stuck behind. Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
On Mon, Nov 05, 2012 at 08:17:54PM +, Alan Cox wrote: IMHO that's the best option for now - if it fixes the specific case of concern right now. It's a trivial interface, it's trivial security (need to check CAP_SYS_RAWIO to flip) and it means that the job can be done properly when there is consensus without a whole extra legacy API stuck behind. Oh yeah, fully agreed. We could just set the sysfs file only writeable by root (basically just setting perm to 0644). Would the CAP_SYS_RAWIO distinction matter? Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
> > It's not really about the lines of code. It adds a new userland > > visible interface. As for the "long" list of commands, it depends on > > how you write it but even if it's textually long it's still very > > simple in terms of actual complexity. > > Sure, but its place is not the kernel. > > As to implementing the ioctl, it's all but trivial. For one thing, you > have to make the block device ioctl op take a "struct file". I have > been asking Al Viro about it for 6 months and I haven't got any answer yet. Just do it - if Al cared he'd have replied about it. > I proposed a way to implement the ultimately flexible solution (BPF) and > you shot it down because it was too complex. Alan is showing you with > multiple examples of why the flexibility would be useful (perhaps nobody > would use it, but the use cases _are_ there), and you are mostly > ignoring them. My feeling too - It feels to me like Tejun is trying to railroad a broken non-solution into the system without regards for anyone else and by simply dismissing any other input. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Il 02/11/2012 18:53, Tejun Heo ha scritto: > Hello, Paolo. > > On Fri, Nov 02, 2012 at 06:49:43PM +0100, Paolo Bonzini wrote: >>> No rule is really absolute. To me, it seems the suggested in-kernel >>> per-device command code filter is both too big for the given problem >> >> Is it? 150 lines of code? The per-class filters would share the first >> two patches with this series, add a long list of commands to filter, and >> the ioctl would be on top of that. > > It's not really about the lines of code. It adds a new userland > visible interface. As for the "long" list of commands, it depends on > how you write it but even if it's textually long it's still very > simple in terms of actual complexity. Sure, but its place is not the kernel. As to implementing the ioctl, it's all but trivial. For one thing, you have to make the block device ioctl op take a "struct file". I have been asking Al Viro about it for 6 months and I haven't got any answer yet. Second, getting a security-sensitive ioctl right is hard, as you demonstrated yourself in this thread by proposing a gapingly insecure approach. Adding a little bit of customization to the current solution may be but a local optimum, but you cannot really get it wrong. >>> while being too limited for much beyond that. >> >> What are the use cases beyond these? AFAIK these were the first two in >> ten years or so... > > If this is such a cold area, why do we want do anything other than the > simplest possible? Because _this_ is the simplest possible. I proposed a way to implement the ultimately flexible solution (BPF) and you shot it down because it was too complex. Alan is showing you with multiple examples of why the flexibility would be useful (perhaps nobody would use it, but the use cases _are_ there), and you are mostly ignoring them. James suggested the sysfs knob, which is not as flexible but is the simplest thing that can work, and was even part of the original design. You are still shooting it down because it is too complex, yet you're proposing to replace one simple mechanism with two; one of which is absolutely inflexible (unlike MMC which only has "ripping" and "burning", other device classes have many use cases), while the other is hard to both implement and get right. Sounds great... Paolo >>> So, if we can get away >>> with adding an ioctl, I personally think that would be a better >>> approach. >> >> I would really prefer to get a green light from Jens/James for per-class >> filters in the kernel (which are worth a few hundred lines of data) >> before implementing that. > > Sure, Jens, James? Guys, come on. > > Thanks. > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Il 02/11/2012 18:53, Tejun Heo ha scritto: Hello, Paolo. On Fri, Nov 02, 2012 at 06:49:43PM +0100, Paolo Bonzini wrote: No rule is really absolute. To me, it seems the suggested in-kernel per-device command code filter is both too big for the given problem Is it? 150 lines of code? The per-class filters would share the first two patches with this series, add a long list of commands to filter, and the ioctl would be on top of that. It's not really about the lines of code. It adds a new userland visible interface. As for the long list of commands, it depends on how you write it but even if it's textually long it's still very simple in terms of actual complexity. Sure, but its place is not the kernel. As to implementing the ioctl, it's all but trivial. For one thing, you have to make the block device ioctl op take a struct file. I have been asking Al Viro about it for 6 months and I haven't got any answer yet. Second, getting a security-sensitive ioctl right is hard, as you demonstrated yourself in this thread by proposing a gapingly insecure approach. Adding a little bit of customization to the current solution may be but a local optimum, but you cannot really get it wrong. while being too limited for much beyond that. What are the use cases beyond these? AFAIK these were the first two in ten years or so... If this is such a cold area, why do we want do anything other than the simplest possible? Because _this_ is the simplest possible. I proposed a way to implement the ultimately flexible solution (BPF) and you shot it down because it was too complex. Alan is showing you with multiple examples of why the flexibility would be useful (perhaps nobody would use it, but the use cases _are_ there), and you are mostly ignoring them. James suggested the sysfs knob, which is not as flexible but is the simplest thing that can work, and was even part of the original design. You are still shooting it down because it is too complex, yet you're proposing to replace one simple mechanism with two; one of which is absolutely inflexible (unlike MMC which only has ripping and burning, other device classes have many use cases), while the other is hard to both implement and get right. Sounds great... Paolo So, if we can get away with adding an ioctl, I personally think that would be a better approach. I would really prefer to get a green light from Jens/James for per-class filters in the kernel (which are worth a few hundred lines of data) before implementing that. Sure, Jens, James? Guys, come on. Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
It's not really about the lines of code. It adds a new userland visible interface. As for the long list of commands, it depends on how you write it but even if it's textually long it's still very simple in terms of actual complexity. Sure, but its place is not the kernel. As to implementing the ioctl, it's all but trivial. For one thing, you have to make the block device ioctl op take a struct file. I have been asking Al Viro about it for 6 months and I haven't got any answer yet. Just do it - if Al cared he'd have replied about it. I proposed a way to implement the ultimately flexible solution (BPF) and you shot it down because it was too complex. Alan is showing you with multiple examples of why the flexibility would be useful (perhaps nobody would use it, but the use cases _are_ there), and you are mostly ignoring them. My feeling too - It feels to me like Tejun is trying to railroad a broken non-solution into the system without regards for anyone else and by simply dismissing any other input. Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
> 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 Thats relevant for /dev/sd but not /dev/sg. Now it might be the sane thing to do is to support BPF filters on /dev/sg only ? > 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. The CD writer one isn't but you tried to dismiss that too. > 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. That requires you have a very trusted hypervisor, that to me is a weaker model because it increases the probability that a hack on a guest of a cloud becomes a hack of the cloud itself at least in terms of quiet data theft from other guests. It's still a minor risk because you've got to break the guest then break the hypervisor from guest virtual ring 0, but the hypervisor is a large target, For a lot of cases being able to do the filtering in the hypervisor makes sense and I don't have a problem with that except that you do need to enforce CAP_SYS_RAWIO on the process setting the flags or you break the kernel capability model. For a hypervisor guest flipping flags at boot thats not a big deal. > > 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. And you've left stuff like CD burning broken still despite people pointing out for years that sorting out the filtering would fix it. > 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. On the CD burning case and on generality I think we are disagreeing in principle. Which isn't to say there is a right answer and one of us is an idiot, just that there are things to consider carefully here. IMHO you are hacking kernel policy by a magic flag to support a specific problem case, not fixing the problem. The question is whether the problem needs fixing 8) Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hello, On Sat, Nov 03, 2012 at 12:19:05AM +, 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
On Fri, 2 Nov 2012 16:58:11 -0700 Tejun Heo wrote: > Hello, Alan. > > On Fri, Nov 02, 2012 at 11:52:43PM +, Alan Cox wrote: > > Really. Can your filter implement it only for certain commands, and only > > for certain vendor specific commands ? Not really because your filter is > > fixed - it has policy in kernel which is the wrong place for device > > specific stuff. > > > > And if you add it to the "fixed" policy how much code and ioctls is that > > to specify ranges by command and pass partitions when they are device > > mapper user space created mappings ? More code than the BPF interface and > > more new APIs. > > 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 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. > > 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 BPF execution is a call to sk_run_filter(). Is your mechanism simpler than that ? It's also been closely audited. > > > For complex/intelligent access policies, kernel isn't the right place > > > to do it anyway > > > > So why are you arguing for kernel policies which is exactly what the > > fixed ones are ? The BPF ones moves the policy to user space ! > > No, it's doing something half-way inbetween in unnecessarily complex > way when you can get by with much simpler mechanism. We are stuck > with the in-kernel whitelist for historical reasons (the proper way > would be implementing burning service in userspace with proper > integration with the rest of userland). The in-kernel whitelist is The burning service is one tiny example. > broken for different classes of devices, so let's fix that in simple > way and give userland a simple mechanism to solve the VM case. Which you haven't done, because you've not provided a filter mechanism that is flexible and takes policy from user space. > It's unfortunate that we have this SG_IO filtering in kernel at all. > Let's please not make it larger. 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. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hello, Alan. On Fri, Nov 02, 2012 at 11:52:43PM +, Alan Cox wrote: > Really. Can your filter implement it only for certain commands, and only > for certain vendor specific commands ? Not really because your filter is > fixed - it has policy in kernel which is the wrong place for device > specific stuff. > > And if you add it to the "fixed" policy how much code and ioctls is that > to specify ranges by command and pass partitions when they are device > mapper user space created mappings ? More code than the BPF interface and > more new APIs. Hmmm? You know which commands you're allowing. You can definitely filter those commands for their ranges. What ioctls? > > At this point, most burning commands are standardized, and optical > > drives are generally on the way out. If you absolutely have to use > > some vendor specific commands, be root. > > 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. I don't think it's unreasonable for this one. It's not like we aim for ultimate flexibility all the time. Everything is a trade-off. BPF filter for vendor-specific CD/DVD burning commands seems way off to me, especially these days. > > > - giving end users minimal access to things like SMART but only on drives > > > where it is safe > > > > End users already have pretty good access to SMART data via udisks and > > it's way more flexible and intelligent than some in-kernel filter. > > Thats a bit Gnome developer - "Our way or the highway". The point of > having a filter is that you put policy in user space. If you don't care > the default filter carries on just working, if you do care you can change > stuff with BPF. We already have it implemented in userspace and it works well. Actually it works better than anything we can do in kernel (e.g. how is the kernel gonna know who's logged in front of the machine has physical access to the hardware?). What's the justification duplicating it worse in kernel? > > Maybe, I don't know. It all sounds highly marginal to me. > > 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. > > For complex/intelligent access policies, kernel isn't the right place > > to do it anyway > > So why are you arguing for kernel policies which is exactly what the > fixed ones are ? The BPF ones moves the policy to user space ! No, it's doing something half-way inbetween in unnecessarily complex way when you can get by with much simpler mechanism. We are stuck with the in-kernel whitelist for historical reasons (the proper way would be implementing burning service in userspace with proper integration with the rest of userland). The in-kernel whitelist is broken for different classes of devices, so let's fix that in simple way and give userland a simple mechanism to solve the VM case. It's unfortunate that we have this SG_IO filtering in kernel at all. Let's please not make it larger. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
> > - giving people access to parts of disks > > Are we gonna implement random range restriction inside a partition > too? If we want to check against partition ranges for allowed SG_IO > commands, by all means, but it can easily be implemented as part of > the fixed filter. Really. Can your filter implement it only for certain commands, and only for certain vendor specific commands ? Not really because your filter is fixed - it has policy in kernel which is the wrong place for device specific stuff. And if you add it to the "fixed" policy how much code and ioctls is that to specify ranges by command and pass partitions when they are device mapper user space created mappings ? More code than the BPF interface and more new APIs. > > - allowing access to specific vendor specific commands on certain > > non-standard CD and DVD drives so they can be used for burning but you > > can't trash them > > At this point, most burning commands are standardized, and optical > drives are generally on the way out. If you absolutely have to use > some vendor specific commands, be root. 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. > > - giving end users minimal access to things like SMART but only on drives > > where it is safe > > End users already have pretty good access to SMART data via udisks and > it's way more flexible and intelligent than some in-kernel filter. Thats a bit Gnome developer - "Our way or the highway". The point of having a filter is that you put policy in user space. If you don't care the default filter carries on just working, if you do care you can change stuff with BPF. > > - giving a user a SCSI disk or partition to play with but preventing them > > issuing weird ass commands that can disrupt other devices in the fabric > > (like drive to drive transfers, some kinds of resets, management > > commands) > > - minimising the ability of a VM to do damage if compromised while > > maximising its raw access to a device > > Maybe, I don't know. It all sounds highly marginal to me. If you are doing virtual machines it is far from marginal. > For complex/intelligent access policies, kernel isn't the right place > to do it anyway So why are you arguing for kernel policies which is exactly what the fixed ones are ? The BPF ones moves the policy to user space ! Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hello, Alan. On Fri, Nov 02, 2012 at 08:48:25PM +, Alan Cox wrote: > If you look back through the archive you'll find people have been > spending a good decade bitching about the lack of filter configurability > and trying to get someone else to fix it. > > The BPF filter is simpler than just about any other implementation > because the tools exist and are used for lots of other things and it has > an API that is precisely defined as well as kernel calls to run the > filter. Sure, if we *have* to go for flexible filtering, BPF would be the right way to do it. I'm just not convinced such flexibility is justified. > Some reasons for it > > - giving people access to parts of disks Are we gonna implement random range restriction inside a partition too? If we want to check against partition ranges for allowed SG_IO commands, by all means, but it can easily be implemented as part of the fixed filter. > - allowing access to specific vendor specific commands on certain > non-standard CD and DVD drives so they can be used for burning but you > can't trash them At this point, most burning commands are standardized, and optical drives are generally on the way out. If you absolutely have to use some vendor specific commands, be root. > - giving end users minimal access to things like SMART but only on drives > where it is safe End users already have pretty good access to SMART data via udisks and it's way more flexible and intelligent than some in-kernel filter. > - giving a user a SCSI disk or partition to play with but preventing them > issuing weird ass commands that can disrupt other devices in the fabric > (like drive to drive transfers, some kinds of resets, management > commands) > - minimising the ability of a VM to do damage if compromised while > maximising its raw access to a device Maybe, I don't know. It all sounds highly marginal to me. For complex/intelligent access policies, kernel isn't the right place to do it anyway - you want strong integration with the rest of the system especially for desktop. Ones which can justify kernel-side filtering would be things which are pretty low-level and require low overhead. I think VMs fall in this category, but then again being able to bypass standard filters seems workable enough for that. Sure, an extra layer of filtering on top could be nice but again I just don't think that's a strong enough justification. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
On Fri, 2 Nov 2012 13:21:31 -0700 Tejun Heo wrote: > Hey, > > On Fri, Nov 02, 2012 at 08:18:24PM +, Alan Cox wrote: > > a - there are lots of cases you want to allow only a subset of commands. > > Care to spell them out. At least the cases Paolo listed should be > served by what's described. > > > b - if you are using a BPF filter which is the obvious way to do it then > > the flexibility comes for free without any extra complexity as the kernel > > provides a generic implementation, and even a JIT for complex cases. > > Yeah, sure, but it's all about what tool to use where and maybe it's > my ignorance about the problem space but it's difficult for me to > believe that we need full-blown BPF filter here when this is the only > activity we've got in a decade. If you look back through the archive you'll find people have been spending a good decade bitching about the lack of filter configurability and trying to get someone else to fix it. The BPF filter is simpler than just about any other implementation because the tools exist and are used for lots of other things and it has an API that is precisely defined as well as kernel calls to run the filter. Some reasons for it - giving people access to parts of disks - allowing access to specific vendor specific commands on certain non-standard CD and DVD drives so they can be used for burning but you can't trash them - giving end users minimal access to things like SMART but only on drives where it is safe - giving a user a SCSI disk or partition to play with but preventing them issuing weird ass commands that can disrupt other devices in the fabric (like drive to drive transfers, some kinds of resets, management commands) - minimising the ability of a VM to do damage if compromised while maximising its raw access to a device Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hey, On Fri, Nov 02, 2012 at 08:18:24PM +, Alan Cox wrote: > a - there are lots of cases you want to allow only a subset of commands. Care to spell them out. At least the cases Paolo listed should be served by what's described. > b - if you are using a BPF filter which is the obvious way to do it then > the flexibility comes for free without any extra complexity as the kernel > provides a generic implementation, and even a JIT for complex cases. Yeah, sure, but it's all about what tool to use where and maybe it's my ignorance about the problem space but it's difficult for me to believe that we need full-blown BPF filter here when this is the only activity we've got in a decade. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
> * Devices are given standard filter matching the device class. Any >!CAP_SYS_RAWIO user can only issue commands allowed by the filter. > > * CAP_SYS_RAWIO can issue an ioctl to disable the filter all >accessors of the fd and transfer it. > > That should be enough, no? No a - there are lots of cases you want to allow only a subset of commands. b - if you are using a BPF filter which is the obvious way to do it then the flexibility comes for free without any extra complexity as the kernel provides a generic implementation, and even a JIT for complex cases. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hello, Paolo. On Fri, Nov 02, 2012 at 06:49:43PM +0100, Paolo Bonzini wrote: > > No rule is really absolute. To me, it seems the suggested in-kernel > > per-device command code filter is both too big for the given problem > > Is it? 150 lines of code? The per-class filters would share the first > two patches with this series, add a long list of commands to filter, and > the ioctl would be on top of that. It's not really about the lines of code. It adds a new userland visible interface. As for the "long" list of commands, it depends on how you write it but even if it's textually long it's still very simple in terms of actual complexity. > > while being too limited for much beyond that. > > What are the use cases beyond these? AFAIK these were the first two in > ten years or so... If this is such a cold area, why do we want do anything other than the simplest possible? > > So, if we can get away > > with adding an ioctl, I personally think that would be a better > > approach. > > I would really prefer to get a green light from Jens/James for per-class > filters in the kernel (which are worth a few hundred lines of data) > before implementing that. Sure, Jens, James? Guys, come on. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Il 02/11/2012 17:51, Tejun Heo ha scritto: >>> > > What disturbs me is that it's a completely new interface to userland >>> > > and at the same a very limited one at that. So, yeah, it's >>> > > bothersome. I personally would prefer SCM_RIGHTS behavior change + >>> > > hard coded filters per device class. >> > >> > I think hard-coded filters are bad (I prefer to move policy to >> > userspace), and SCM_RIGHTS without a ioctl is out of question, really. > No rule is really absolute. To me, it seems the suggested in-kernel > per-device command code filter is both too big for the given problem Is it? 150 lines of code? The per-class filters would share the first two patches with this series, add a long list of commands to filter, and the ioctl would be on top of that. Long lists are better kept in configuration files than in kernel sources; not to mention the higher cost of getting the API wrong for a ioctl vs. sysfs. > while being too limited for much beyond that. What are the use cases beyond these? AFAIK these were the first two in ten years or so... > So, if we can get away > with adding an ioctl, I personally think that would be a better > approach. I would really prefer to get a green light from Jens/James for per-class filters in the kernel (which are worth a few hundred lines of data) before implementing that. Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hey, Alan. On Fri, Nov 02, 2012 at 05:21:45PM +, Alan Cox wrote: > That also means that a normal app running as superuser for some reason > would set its user filter and any accidentally inherited descriptors will > be less dangerous as the are today. It also means a CAP_SYS_RAWIO capable > app can still use filters itself as good programming practise. > > It effectively means you have to deliberately and intentionally set up an > 'inherited' extra rights case. The last part, I agree, but in general I think what you're describing is way too elaborate for the problem at hand. It's like adding arbitrary range-filter for /dev/sdX which can be overridden by userland. You sure can find use case for such thing if you try hard enough, but it's way over-engineered nonetheless. I don't think we're addressing huge range and number of use cases here and would much prefer to keep it as simple as possible. * Devices are given standard filter matching the device class. Any !CAP_SYS_RAWIO user can only issue commands allowed by the filter. * CAP_SYS_RAWIO can issue an ioctl to disable the filter all accessors of the fd and transfer it. That should be enough, no? Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
> > Not a good model. Any removal of filters and passing them to a task > > should be explicit. The behaviour really ought to be to permit the > > intentional setting of explicit filters then passing them, not touch the > > default behaviour. > > Yeah, well, then I guess it'll have to be a separate ioctl to switch > SG_IO for !root users. My first thought would be to have the basic behaviour as allowed IFF passes user filter && CAP_SYS_RAWIO || passes 'root' filter that allows untrusted to also push unprivileged filters for their own purposes (consider things like exokernel experiments or just trying to ensure a raw disk emulation doesn't go wrong). The default user feature would be 'allow anything'. then add a way to 'set' the root filter only if you have CAP_SYS_RAWIO with the default 'root' filter being the current hardcoded filter. That also means that a normal app running as superuser for some reason would set its user filter and any accidentally inherited descriptors will be less dangerous as the are today. It also means a CAP_SYS_RAWIO capable app can still use filters itself as good programming practise. It effectively means you have to deliberately and intentionally set up an 'inherited' extra rights case. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hey, Paolo. On Fri, Nov 02, 2012 at 03:49:02PM +0100, Paolo Bonzini wrote: > > Yeah, I get that it's a behavior change, but would that be a problem? > > Worse, it's a potential security hole because previously you'd get > filtering and now you wouldn't. > > Considering that SCM_RIGHTS is usually used to transfer a file > descriptor from a privileged process to an unprivileged one, I'd be very > worried of that. Yeah, I know it's a security thing, was wondering how bad it was. So, if we choose this, I guess we'll need an ioctl to switch userland SG_IO filtering. > > What disturbs me is that it's a completely new interface to userland > > and at the same a very limited one at that. So, yeah, it's > > bothersome. I personally would prefer SCM_RIGHTS behavior change + > > hard coded filters per device class. > > I think hard-coded filters are bad (I prefer to move policy to > userspace), and SCM_RIGHTS without a ioctl is out of question, really. No rule is really absolute. To me, it seems the suggested in-kernel per-device command code filter is both too big for the given problem while being too limited for much beyond that. So, if we can get away with adding an ioctl, I personally think that would be a better approach. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hey, Alan, Paolo. On Fri, Nov 02, 2012 at 03:35:30PM +, Alan Cox wrote: > > >> That would be a change with respect to what we have now. After > > >> transferring a root-opened (better: CAP_SYS_RAWIO-opened) file > > >> descriptor to an unprivileged process your SG_IO commands get > > >> filtered. So a ioctl is needed if you want to rely on SCM_RIGHTS. > > > > > > Yeah, I get that it's a behavior change, but would that be a problem? > > > > Worse, it's a potential security hole because previously you'd get > > filtering and now you wouldn't. > > > > Considering that SCM_RIGHTS is usually used to transfer a file > > descriptor from a privileged process to an unprivileged one, I'd be very > > worried of that. > > In other contexts you inherit file handles via exec and having a "root > opened so its special" model is bad. Historically it led to things like > the rlogin/rsh hacks on SunOS and friends where a program run by the rsh > daemon got a root opened socket as its stdin/out and could issue ifconfig > ioctls on it at will. > > Not a good model. Any removal of filters and passing them to a task > should be explicit. The behaviour really ought to be to permit the > intentional setting of explicit filters then passing them, not touch the > default behaviour. Yeah, well, then I guess it'll have to be a separate ioctl to switch SG_IO for !root users. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
On Fri, 02 Nov 2012 15:49:02 +0100 Paolo Bonzini wrote: > Il 31/10/2012 22:22, Tejun Heo ha scritto: > > Hello, Paolo. > > > > On Thu, Oct 25, 2012 at 02:35:20PM -0400, Paolo Bonzini wrote: > >>> Disabling filters if opened by root and tranfering via SCM_RIGHTS > >>> would be the simplest interface-wise (there's no new interface at > >>> all). Would that be too dangerous security-wise? > >> > >> That would be a change with respect to what we have now. After > >> transferring a root-opened (better: CAP_SYS_RAWIO-opened) file > >> descriptor to an unprivileged process your SG_IO commands get > >> filtered. So a ioctl is needed if you want to rely on SCM_RIGHTS. > > > > Yeah, I get that it's a behavior change, but would that be a problem? > > Worse, it's a potential security hole because previously you'd get > filtering and now you wouldn't. > > Considering that SCM_RIGHTS is usually used to transfer a file > descriptor from a privileged process to an unprivileged one, I'd be very > worried of that. In other contexts you inherit file handles via exec and having a "root opened so its special" model is bad. Historically it led to things like the rlogin/rsh hacks on SunOS and friends where a program run by the rsh daemon got a root opened socket as its stdin/out and could issue ifconfig ioctls on it at will. Not a good model. Any removal of filters and passing them to a task should be explicit. The behaviour really ought to be to permit the intentional setting of explicit filters then passing them, not touch the default behaviour. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Il 31/10/2012 22:22, Tejun Heo ha scritto: > Hello, Paolo. > > On Thu, Oct 25, 2012 at 02:35:20PM -0400, Paolo Bonzini wrote: >>> Disabling filters if opened by root and tranfering via SCM_RIGHTS >>> would be the simplest interface-wise (there's no new interface at >>> all). Would that be too dangerous security-wise? >> >> That would be a change with respect to what we have now. After >> transferring a root-opened (better: CAP_SYS_RAWIO-opened) file >> descriptor to an unprivileged process your SG_IO commands get >> filtered. So a ioctl is needed if you want to rely on SCM_RIGHTS. > > Yeah, I get that it's a behavior change, but would that be a problem? Worse, it's a potential security hole because previously you'd get filtering and now you wouldn't. Considering that SCM_RIGHTS is usually used to transfer a file descriptor from a privileged process to an unprivileged one, I'd be very worried of that. >>> I guess I just feel quite reluctant to expose another rather obscure >>> userland configurable in-kernel filter and at the same time I'm not >>> sure whether this is flexible enough. What if a device is shared by >>> multiple virtual machines which are trusted at different levels? >> >> No, you just don't do that. If a device is passed through to virtual >> machines, it is between similar virtual machines (for some definition >> of similar). The only case where you have this sharing is in practice >> if either the device is read-only (my patch does give you a basic >> two-level filtering, with two separate bitmaps for RO and RW) or if you >> allow persistent reservations (which is as close to full trust as you >> can get). > > What disturbs me is that it's a completely new interface to userland > and at the same a very limited one at that. So, yeah, it's > bothersome. I personally would prefer SCM_RIGHTS behavior change + > hard coded filters per device class. I think hard-coded filters are bad (I prefer to move policy to userspace), and SCM_RIGHTS without a ioctl is out of question, really. > But, I'd really like to hear what other guys are thinking. Jens? > Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? :P :P Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Il 31/10/2012 22:22, Tejun Heo ha scritto: Hello, Paolo. On Thu, Oct 25, 2012 at 02:35:20PM -0400, Paolo Bonzini wrote: Disabling filters if opened by root and tranfering via SCM_RIGHTS would be the simplest interface-wise (there's no new interface at all). Would that be too dangerous security-wise? That would be a change with respect to what we have now. After transferring a root-opened (better: CAP_SYS_RAWIO-opened) file descriptor to an unprivileged process your SG_IO commands get filtered. So a ioctl is needed if you want to rely on SCM_RIGHTS. Yeah, I get that it's a behavior change, but would that be a problem? Worse, it's a potential security hole because previously you'd get filtering and now you wouldn't. Considering that SCM_RIGHTS is usually used to transfer a file descriptor from a privileged process to an unprivileged one, I'd be very worried of that. I guess I just feel quite reluctant to expose another rather obscure userland configurable in-kernel filter and at the same time I'm not sure whether this is flexible enough. What if a device is shared by multiple virtual machines which are trusted at different levels? No, you just don't do that. If a device is passed through to virtual machines, it is between similar virtual machines (for some definition of similar). The only case where you have this sharing is in practice if either the device is read-only (my patch does give you a basic two-level filtering, with two separate bitmaps for RO and RW) or if you allow persistent reservations (which is as close to full trust as you can get). What disturbs me is that it's a completely new interface to userland and at the same a very limited one at that. So, yeah, it's bothersome. I personally would prefer SCM_RIGHTS behavior change + hard coded filters per device class. I think hard-coded filters are bad (I prefer to move policy to userspace), and SCM_RIGHTS without a ioctl is out of question, really. But, I'd really like to hear what other guys are thinking. Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? :P :P Paolo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
On Fri, 02 Nov 2012 15:49:02 +0100 Paolo Bonzini pbonz...@redhat.com wrote: Il 31/10/2012 22:22, Tejun Heo ha scritto: Hello, Paolo. On Thu, Oct 25, 2012 at 02:35:20PM -0400, Paolo Bonzini wrote: Disabling filters if opened by root and tranfering via SCM_RIGHTS would be the simplest interface-wise (there's no new interface at all). Would that be too dangerous security-wise? That would be a change with respect to what we have now. After transferring a root-opened (better: CAP_SYS_RAWIO-opened) file descriptor to an unprivileged process your SG_IO commands get filtered. So a ioctl is needed if you want to rely on SCM_RIGHTS. Yeah, I get that it's a behavior change, but would that be a problem? Worse, it's a potential security hole because previously you'd get filtering and now you wouldn't. Considering that SCM_RIGHTS is usually used to transfer a file descriptor from a privileged process to an unprivileged one, I'd be very worried of that. In other contexts you inherit file handles via exec and having a root opened so its special model is bad. Historically it led to things like the rlogin/rsh hacks on SunOS and friends where a program run by the rsh daemon got a root opened socket as its stdin/out and could issue ifconfig ioctls on it at will. Not a good model. Any removal of filters and passing them to a task should be explicit. The behaviour really ought to be to permit the intentional setting of explicit filters then passing them, not touch the default behaviour. Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hey, Alan, Paolo. On Fri, Nov 02, 2012 at 03:35:30PM +, Alan Cox wrote: That would be a change with respect to what we have now. After transferring a root-opened (better: CAP_SYS_RAWIO-opened) file descriptor to an unprivileged process your SG_IO commands get filtered. So a ioctl is needed if you want to rely on SCM_RIGHTS. Yeah, I get that it's a behavior change, but would that be a problem? Worse, it's a potential security hole because previously you'd get filtering and now you wouldn't. Considering that SCM_RIGHTS is usually used to transfer a file descriptor from a privileged process to an unprivileged one, I'd be very worried of that. In other contexts you inherit file handles via exec and having a root opened so its special model is bad. Historically it led to things like the rlogin/rsh hacks on SunOS and friends where a program run by the rsh daemon got a root opened socket as its stdin/out and could issue ifconfig ioctls on it at will. Not a good model. Any removal of filters and passing them to a task should be explicit. The behaviour really ought to be to permit the intentional setting of explicit filters then passing them, not touch the default behaviour. Yeah, well, then I guess it'll have to be a separate ioctl to switch SG_IO for !root users. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hey, Paolo. On Fri, Nov 02, 2012 at 03:49:02PM +0100, Paolo Bonzini wrote: Yeah, I get that it's a behavior change, but would that be a problem? Worse, it's a potential security hole because previously you'd get filtering and now you wouldn't. Considering that SCM_RIGHTS is usually used to transfer a file descriptor from a privileged process to an unprivileged one, I'd be very worried of that. Yeah, I know it's a security thing, was wondering how bad it was. So, if we choose this, I guess we'll need an ioctl to switch userland SG_IO filtering. What disturbs me is that it's a completely new interface to userland and at the same a very limited one at that. So, yeah, it's bothersome. I personally would prefer SCM_RIGHTS behavior change + hard coded filters per device class. I think hard-coded filters are bad (I prefer to move policy to userspace), and SCM_RIGHTS without a ioctl is out of question, really. No rule is really absolute. To me, it seems the suggested in-kernel per-device command code filter is both too big for the given problem while being too limited for much beyond that. So, if we can get away with adding an ioctl, I personally think that would be a better approach. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Not a good model. Any removal of filters and passing them to a task should be explicit. The behaviour really ought to be to permit the intentional setting of explicit filters then passing them, not touch the default behaviour. Yeah, well, then I guess it'll have to be a separate ioctl to switch SG_IO for !root users. My first thought would be to have the basic behaviour as allowed IFF passes user filter CAP_SYS_RAWIO || passes 'root' filter that allows untrusted to also push unprivileged filters for their own purposes (consider things like exokernel experiments or just trying to ensure a raw disk emulation doesn't go wrong). The default user feature would be 'allow anything'. then add a way to 'set' the root filter only if you have CAP_SYS_RAWIO with the default 'root' filter being the current hardcoded filter. That also means that a normal app running as superuser for some reason would set its user filter and any accidentally inherited descriptors will be less dangerous as the are today. It also means a CAP_SYS_RAWIO capable app can still use filters itself as good programming practise. It effectively means you have to deliberately and intentionally set up an 'inherited' extra rights case. Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hey, Alan. On Fri, Nov 02, 2012 at 05:21:45PM +, Alan Cox wrote: That also means that a normal app running as superuser for some reason would set its user filter and any accidentally inherited descriptors will be less dangerous as the are today. It also means a CAP_SYS_RAWIO capable app can still use filters itself as good programming practise. It effectively means you have to deliberately and intentionally set up an 'inherited' extra rights case. The last part, I agree, but in general I think what you're describing is way too elaborate for the problem at hand. It's like adding arbitrary range-filter for /dev/sdX which can be overridden by userland. You sure can find use case for such thing if you try hard enough, but it's way over-engineered nonetheless. I don't think we're addressing huge range and number of use cases here and would much prefer to keep it as simple as possible. * Devices are given standard filter matching the device class. Any !CAP_SYS_RAWIO user can only issue commands allowed by the filter. * CAP_SYS_RAWIO can issue an ioctl to disable the filter all accessors of the fd and transfer it. That should be enough, no? Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Il 02/11/2012 17:51, Tejun Heo ha scritto: What disturbs me is that it's a completely new interface to userland and at the same a very limited one at that. So, yeah, it's bothersome. I personally would prefer SCM_RIGHTS behavior change + hard coded filters per device class. I think hard-coded filters are bad (I prefer to move policy to userspace), and SCM_RIGHTS without a ioctl is out of question, really. No rule is really absolute. To me, it seems the suggested in-kernel per-device command code filter is both too big for the given problem Is it? 150 lines of code? The per-class filters would share the first two patches with this series, add a long list of commands to filter, and the ioctl would be on top of that. Long lists are better kept in configuration files than in kernel sources; not to mention the higher cost of getting the API wrong for a ioctl vs. sysfs. while being too limited for much beyond that. What are the use cases beyond these? AFAIK these were the first two in ten years or so... So, if we can get away with adding an ioctl, I personally think that would be a better approach. I would really prefer to get a green light from Jens/James for per-class filters in the kernel (which are worth a few hundred lines of data) before implementing that. Paolo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hello, Paolo. On Fri, Nov 02, 2012 at 06:49:43PM +0100, Paolo Bonzini wrote: No rule is really absolute. To me, it seems the suggested in-kernel per-device command code filter is both too big for the given problem Is it? 150 lines of code? The per-class filters would share the first two patches with this series, add a long list of commands to filter, and the ioctl would be on top of that. It's not really about the lines of code. It adds a new userland visible interface. As for the long list of commands, it depends on how you write it but even if it's textually long it's still very simple in terms of actual complexity. while being too limited for much beyond that. What are the use cases beyond these? AFAIK these were the first two in ten years or so... If this is such a cold area, why do we want do anything other than the simplest possible? So, if we can get away with adding an ioctl, I personally think that would be a better approach. I would really prefer to get a green light from Jens/James for per-class filters in the kernel (which are worth a few hundred lines of data) before implementing that. Sure, Jens, James? Guys, come on. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
* Devices are given standard filter matching the device class. Any !CAP_SYS_RAWIO user can only issue commands allowed by the filter. * CAP_SYS_RAWIO can issue an ioctl to disable the filter all accessors of the fd and transfer it. That should be enough, no? No a - there are lots of cases you want to allow only a subset of commands. b - if you are using a BPF filter which is the obvious way to do it then the flexibility comes for free without any extra complexity as the kernel provides a generic implementation, and even a JIT for complex cases. Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hey, On Fri, Nov 02, 2012 at 08:18:24PM +, Alan Cox wrote: a - there are lots of cases you want to allow only a subset of commands. Care to spell them out. At least the cases Paolo listed should be served by what's described. b - if you are using a BPF filter which is the obvious way to do it then the flexibility comes for free without any extra complexity as the kernel provides a generic implementation, and even a JIT for complex cases. Yeah, sure, but it's all about what tool to use where and maybe it's my ignorance about the problem space but it's difficult for me to believe that we need full-blown BPF filter here when this is the only activity we've got in a decade. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
On Fri, 2 Nov 2012 13:21:31 -0700 Tejun Heo t...@kernel.org wrote: Hey, On Fri, Nov 02, 2012 at 08:18:24PM +, Alan Cox wrote: a - there are lots of cases you want to allow only a subset of commands. Care to spell them out. At least the cases Paolo listed should be served by what's described. b - if you are using a BPF filter which is the obvious way to do it then the flexibility comes for free without any extra complexity as the kernel provides a generic implementation, and even a JIT for complex cases. Yeah, sure, but it's all about what tool to use where and maybe it's my ignorance about the problem space but it's difficult for me to believe that we need full-blown BPF filter here when this is the only activity we've got in a decade. If you look back through the archive you'll find people have been spending a good decade bitching about the lack of filter configurability and trying to get someone else to fix it. The BPF filter is simpler than just about any other implementation because the tools exist and are used for lots of other things and it has an API that is precisely defined as well as kernel calls to run the filter. Some reasons for it - giving people access to parts of disks - allowing access to specific vendor specific commands on certain non-standard CD and DVD drives so they can be used for burning but you can't trash them - giving end users minimal access to things like SMART but only on drives where it is safe - giving a user a SCSI disk or partition to play with but preventing them issuing weird ass commands that can disrupt other devices in the fabric (like drive to drive transfers, some kinds of resets, management commands) - minimising the ability of a VM to do damage if compromised while maximising its raw access to a device Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hello, Alan. On Fri, Nov 02, 2012 at 08:48:25PM +, Alan Cox wrote: If you look back through the archive you'll find people have been spending a good decade bitching about the lack of filter configurability and trying to get someone else to fix it. The BPF filter is simpler than just about any other implementation because the tools exist and are used for lots of other things and it has an API that is precisely defined as well as kernel calls to run the filter. Sure, if we *have* to go for flexible filtering, BPF would be the right way to do it. I'm just not convinced such flexibility is justified. Some reasons for it - giving people access to parts of disks Are we gonna implement random range restriction inside a partition too? If we want to check against partition ranges for allowed SG_IO commands, by all means, but it can easily be implemented as part of the fixed filter. - allowing access to specific vendor specific commands on certain non-standard CD and DVD drives so they can be used for burning but you can't trash them At this point, most burning commands are standardized, and optical drives are generally on the way out. If you absolutely have to use some vendor specific commands, be root. - giving end users minimal access to things like SMART but only on drives where it is safe End users already have pretty good access to SMART data via udisks and it's way more flexible and intelligent than some in-kernel filter. - giving a user a SCSI disk or partition to play with but preventing them issuing weird ass commands that can disrupt other devices in the fabric (like drive to drive transfers, some kinds of resets, management commands) - minimising the ability of a VM to do damage if compromised while maximising its raw access to a device Maybe, I don't know. It all sounds highly marginal to me. For complex/intelligent access policies, kernel isn't the right place to do it anyway - you want strong integration with the rest of the system especially for desktop. Ones which can justify kernel-side filtering would be things which are pretty low-level and require low overhead. I think VMs fall in this category, but then again being able to bypass standard filters seems workable enough for that. Sure, an extra layer of filtering on top could be nice but again I just don't think that's a strong enough justification. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
- giving people access to parts of disks Are we gonna implement random range restriction inside a partition too? If we want to check against partition ranges for allowed SG_IO commands, by all means, but it can easily be implemented as part of the fixed filter. Really. Can your filter implement it only for certain commands, and only for certain vendor specific commands ? Not really because your filter is fixed - it has policy in kernel which is the wrong place for device specific stuff. And if you add it to the fixed policy how much code and ioctls is that to specify ranges by command and pass partitions when they are device mapper user space created mappings ? More code than the BPF interface and more new APIs. - allowing access to specific vendor specific commands on certain non-standard CD and DVD drives so they can be used for burning but you can't trash them At this point, most burning commands are standardized, and optical drives are generally on the way out. If you absolutely have to use some vendor specific commands, be root. 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. - giving end users minimal access to things like SMART but only on drives where it is safe End users already have pretty good access to SMART data via udisks and it's way more flexible and intelligent than some in-kernel filter. Thats a bit Gnome developer - Our way or the highway. The point of having a filter is that you put policy in user space. If you don't care the default filter carries on just working, if you do care you can change stuff with BPF. - giving a user a SCSI disk or partition to play with but preventing them issuing weird ass commands that can disrupt other devices in the fabric (like drive to drive transfers, some kinds of resets, management commands) - minimising the ability of a VM to do damage if compromised while maximising its raw access to a device Maybe, I don't know. It all sounds highly marginal to me. If you are doing virtual machines it is far from marginal. For complex/intelligent access policies, kernel isn't the right place to do it anyway So why are you arguing for kernel policies which is exactly what the fixed ones are ? The BPF ones moves the policy to user space ! Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hello, Alan. On Fri, Nov 02, 2012 at 11:52:43PM +, Alan Cox wrote: Really. Can your filter implement it only for certain commands, and only for certain vendor specific commands ? Not really because your filter is fixed - it has policy in kernel which is the wrong place for device specific stuff. And if you add it to the fixed policy how much code and ioctls is that to specify ranges by command and pass partitions when they are device mapper user space created mappings ? More code than the BPF interface and more new APIs. Hmmm? You know which commands you're allowing. You can definitely filter those commands for their ranges. What ioctls? At this point, most burning commands are standardized, and optical drives are generally on the way out. If you absolutely have to use some vendor specific commands, be root. 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. I don't think it's unreasonable for this one. It's not like we aim for ultimate flexibility all the time. Everything is a trade-off. BPF filter for vendor-specific CD/DVD burning commands seems way off to me, especially these days. - giving end users minimal access to things like SMART but only on drives where it is safe End users already have pretty good access to SMART data via udisks and it's way more flexible and intelligent than some in-kernel filter. Thats a bit Gnome developer - Our way or the highway. The point of having a filter is that you put policy in user space. If you don't care the default filter carries on just working, if you do care you can change stuff with BPF. We already have it implemented in userspace and it works well. Actually it works better than anything we can do in kernel (e.g. how is the kernel gonna know who's logged in front of the machine has physical access to the hardware?). What's the justification duplicating it worse in kernel? Maybe, I don't know. It all sounds highly marginal to me. 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. For complex/intelligent access policies, kernel isn't the right place to do it anyway So why are you arguing for kernel policies which is exactly what the fixed ones are ? The BPF ones moves the policy to user space ! No, it's doing something half-way inbetween in unnecessarily complex way when you can get by with much simpler mechanism. We are stuck with the in-kernel whitelist for historical reasons (the proper way would be implementing burning service in userspace with proper integration with the rest of userland). The in-kernel whitelist is broken for different classes of devices, so let's fix that in simple way and give userland a simple mechanism to solve the VM case. It's unfortunate that we have this SG_IO filtering in kernel at all. Let's please not make it larger. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
On Fri, 2 Nov 2012 16:58:11 -0700 Tejun Heo t...@kernel.org wrote: Hello, Alan. On Fri, Nov 02, 2012 at 11:52:43PM +, Alan Cox wrote: Really. Can your filter implement it only for certain commands, and only for certain vendor specific commands ? Not really because your filter is fixed - it has policy in kernel which is the wrong place for device specific stuff. And if you add it to the fixed policy how much code and ioctls is that to specify ranges by command and pass partitions when they are device mapper user space created mappings ? More code than the BPF interface and more new APIs. 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 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. 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 BPF execution is a call to sk_run_filter(). Is your mechanism simpler than that ? It's also been closely audited. For complex/intelligent access policies, kernel isn't the right place to do it anyway So why are you arguing for kernel policies which is exactly what the fixed ones are ? The BPF ones moves the policy to user space ! No, it's doing something half-way inbetween in unnecessarily complex way when you can get by with much simpler mechanism. We are stuck with the in-kernel whitelist for historical reasons (the proper way would be implementing burning service in userspace with proper integration with the rest of userland). The in-kernel whitelist is The burning service is one tiny example. broken for different classes of devices, so let's fix that in simple way and give userland a simple mechanism to solve the VM case. Which you haven't done, because you've not provided a filter mechanism that is flexible and takes policy from user space. It's unfortunate that we have this SG_IO filtering in kernel at all. Let's please not make it larger. 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. Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hello, On Sat, Nov 03, 2012 at 12:19:05AM +, 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
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 Thats relevant for /dev/sd but not /dev/sg. Now it might be the sane thing to do is to support BPF filters on /dev/sg only ? 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. The CD writer one isn't but you tried to dismiss that too. 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. That requires you have a very trusted hypervisor, that to me is a weaker model because it increases the probability that a hack on a guest of a cloud becomes a hack of the cloud itself at least in terms of quiet data theft from other guests. It's still a minor risk because you've got to break the guest then break the hypervisor from guest virtual ring 0, but the hypervisor is a large target, For a lot of cases being able to do the filtering in the hypervisor makes sense and I don't have a problem with that except that you do need to enforce CAP_SYS_RAWIO on the process setting the flags or you break the kernel capability model. For a hypervisor guest flipping flags at boot thats not a big deal. 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. And you've left stuff like CD burning broken still despite people pointing out for years that sorting out the filtering would fix it. 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. On the CD burning case and on generality I think we are disagreeing in principle. Which isn't to say there is a right answer and one of us is an idiot, just that there are things to consider carefully here. IMHO you are hacking kernel policy by a magic flag to support a specific problem case, not fixing the problem. The question is whether the problem needs fixing 8) Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hello, Paolo. On Thu, Oct 25, 2012 at 02:35:20PM -0400, Paolo Bonzini wrote: > > Disabling filters if opened by root and tranfering via SCM_RIGHTS > > would be the simplest interface-wise (there's no new interface at > > all). Would that be too dangerous security-wise? > > That would be a change with respect to what we have now. After > transferring a root-opened (better: CAP_SYS_RAWIO-opened) file > descriptor to an unprivileged process your SG_IO commands get > filtered. So a ioctl is needed if you want to rely on SCM_RIGHTS. Yeah, I get that it's a behavior change, but would that be a problem? > > I guess I just feel quite reluctant to expose another rather obscure > > userland configurable in-kernel filter and at the same time I'm not > > sure whether this is flexible enough. What if a device is shared by > > multiple virtual machines which are trusted at different levels? > > No, you just don't do that. If a device is passed through to virtual > machines, it is between similar virtual machines (for some definition > of similar). The only case where you have this sharing is in practice > if either the device is read-only (my patch does give you a basic > two-level filtering, with two separate bitmaps for RO and RW) or if you > allow persistent reservations (which is as close to full trust as you > can get). What disturbs me is that it's a completely new interface to userland and at the same a very limited one at that. So, yeah, it's bothersome. I personally would prefer SCM_RIGHTS behavior change + hard coded filters per device class. But, I'd really like to hear what other guys are thinking. Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? :P Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Ping? Paolo Il 25/10/2012 20:35, Paolo Bonzini ha scritto: >> On Thu, Oct 25, 2012 at 09:37:39AM +0200, Paolo Bonzini wrote: >>> Il 24/10/2012 18:47, Tejun Heo ha scritto: So, I'm still not convinced we need to go forward with full configurability. All use cases you described can be covered with per-class static filters + simple override switch to disable all, which would result in a lot simpler implementation w/ much smaller userland interface. >>> >>> I'm not sure the userland interface would be smaller, and it would >>> be more complex to get right: >>> >>> 1) how do you override the default? ioctl+SCM_RIGHTS or sysfs? >> >> Disabling filters if opened by root and tranfering via SCM_RIGHTS >> would be the simplest interface-wise (there's no new interface at >> all). Would that be too dangerous security-wise? > > That would be a change with respect to what we have now. After > transferring a root-opened (better: CAP_SYS_RAWIO-opened) file > descriptor to an unprivileged process your SG_IO commands get > filtered. So a ioctl is needed if you want to rely on SCM_RIGHTS. > >>> 2) do you need to override the default to "no access", "full >>> access" and "default access", or is a binary knob (default >>> access/full access) sufficient? >> >> Default / full should be enough, no? > > If a ioctl has to be added, I'd rather have at least none/full/default. > >>> 3) what capabilities control the setting? >> >> CAP_SYS_RAWIO seems to be a pretty good fit. > > Yes, for a ioctl it is (for sysfs CAP_SYS_ADMIN is better IMHO). > >> I guess I just feel quite reluctant to expose another rather obscure >> userland configurable in-kernel filter and at the same time I'm not >> sure whether this is flexible enough. What if a device is shared by >> multiple virtual machines which are trusted at different levels? > > No, you just don't do that. If a device is passed through to virtual > machines, it is between similar virtual machines (for some definition > of similar). The only case where you have this sharing is in practice > if either the device is read-only (my patch does give you a basic > two-level filtering, with two separate bitmaps for RO and RW) or if you > allow persistent reservations (which is as close to full trust as you > can get). > >> I'm not trying to block it at all cost but let's make sure we looked >> into most possibilities before (re)adding this userland visible >> interface. > > Sure, understood. :) > >> Jens, James, what do you guys think? > > Paolo > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Ping? Paolo Il 25/10/2012 20:35, Paolo Bonzini ha scritto: On Thu, Oct 25, 2012 at 09:37:39AM +0200, Paolo Bonzini wrote: Il 24/10/2012 18:47, Tejun Heo ha scritto: So, I'm still not convinced we need to go forward with full configurability. All use cases you described can be covered with per-class static filters + simple override switch to disable all, which would result in a lot simpler implementation w/ much smaller userland interface. I'm not sure the userland interface would be smaller, and it would be more complex to get right: 1) how do you override the default? ioctl+SCM_RIGHTS or sysfs? Disabling filters if opened by root and tranfering via SCM_RIGHTS would be the simplest interface-wise (there's no new interface at all). Would that be too dangerous security-wise? That would be a change with respect to what we have now. After transferring a root-opened (better: CAP_SYS_RAWIO-opened) file descriptor to an unprivileged process your SG_IO commands get filtered. So a ioctl is needed if you want to rely on SCM_RIGHTS. 2) do you need to override the default to no access, full access and default access, or is a binary knob (default access/full access) sufficient? Default / full should be enough, no? If a ioctl has to be added, I'd rather have at least none/full/default. 3) what capabilities control the setting? CAP_SYS_RAWIO seems to be a pretty good fit. Yes, for a ioctl it is (for sysfs CAP_SYS_ADMIN is better IMHO). I guess I just feel quite reluctant to expose another rather obscure userland configurable in-kernel filter and at the same time I'm not sure whether this is flexible enough. What if a device is shared by multiple virtual machines which are trusted at different levels? No, you just don't do that. If a device is passed through to virtual machines, it is between similar virtual machines (for some definition of similar). The only case where you have this sharing is in practice if either the device is read-only (my patch does give you a basic two-level filtering, with two separate bitmaps for RO and RW) or if you allow persistent reservations (which is as close to full trust as you can get). I'm not trying to block it at all cost but let's make sure we looked into most possibilities before (re)adding this userland visible interface. Sure, understood. :) Jens, James, what do you guys think? Paolo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
Hello, Paolo. On Thu, Oct 25, 2012 at 02:35:20PM -0400, Paolo Bonzini wrote: Disabling filters if opened by root and tranfering via SCM_RIGHTS would be the simplest interface-wise (there's no new interface at all). Would that be too dangerous security-wise? That would be a change with respect to what we have now. After transferring a root-opened (better: CAP_SYS_RAWIO-opened) file descriptor to an unprivileged process your SG_IO commands get filtered. So a ioctl is needed if you want to rely on SCM_RIGHTS. Yeah, I get that it's a behavior change, but would that be a problem? I guess I just feel quite reluctant to expose another rather obscure userland configurable in-kernel filter and at the same time I'm not sure whether this is flexible enough. What if a device is shared by multiple virtual machines which are trusted at different levels? No, you just don't do that. If a device is passed through to virtual machines, it is between similar virtual machines (for some definition of similar). The only case where you have this sharing is in practice if either the device is read-only (my patch does give you a basic two-level filtering, with two separate bitmaps for RO and RW) or if you allow persistent reservations (which is as close to full trust as you can get). What disturbs me is that it's a completely new interface to userland and at the same a very limited one at that. So, yeah, it's bothersome. I personally would prefer SCM_RIGHTS behavior change + hard coded filters per device class. But, I'd really like to hear what other guys are thinking. Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? Jens? :P Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
> On Thu, Oct 25, 2012 at 09:37:39AM +0200, Paolo Bonzini wrote: > > Il 24/10/2012 18:47, Tejun Heo ha scritto: > > > So, I'm still not convinced we need to go forward with full > > > configurability. All use cases you described can be covered with > > > per-class static filters + simple override switch to disable all, > > > which would result in a lot simpler implementation w/ much > > > smaller userland interface. > > > > I'm not sure the userland interface would be smaller, and it would > > be more complex to get right: > > > > 1) how do you override the default? ioctl+SCM_RIGHTS or sysfs? > > Disabling filters if opened by root and tranfering via SCM_RIGHTS > would be the simplest interface-wise (there's no new interface at > all). Would that be too dangerous security-wise? That would be a change with respect to what we have now. After transferring a root-opened (better: CAP_SYS_RAWIO-opened) file descriptor to an unprivileged process your SG_IO commands get filtered. So a ioctl is needed if you want to rely on SCM_RIGHTS. > > 2) do you need to override the default to "no access", "full > > access" and "default access", or is a binary knob (default > > access/full access) sufficient? > > Default / full should be enough, no? If a ioctl has to be added, I'd rather have at least none/full/default. > > 3) what capabilities control the setting? > > CAP_SYS_RAWIO seems to be a pretty good fit. Yes, for a ioctl it is (for sysfs CAP_SYS_ADMIN is better IMHO). > I guess I just feel quite reluctant to expose another rather obscure > userland configurable in-kernel filter and at the same time I'm not > sure whether this is flexible enough. What if a device is shared by > multiple virtual machines which are trusted at different levels? No, you just don't do that. If a device is passed through to virtual machines, it is between similar virtual machines (for some definition of similar). The only case where you have this sharing is in practice if either the device is read-only (my patch does give you a basic two-level filtering, with two separate bitmaps for RO and RW) or if you allow persistent reservations (which is as close to full trust as you can get). > I'm not trying to block it at all cost but let's make sure we looked > into most possibilities before (re)adding this userland visible > interface. Sure, understood. :) > Jens, James, what do you guys think? Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
(restoring cc lists) Hey, Paolo. On Thu, Oct 25, 2012 at 09:37:39AM +0200, Paolo Bonzini wrote: > Il 24/10/2012 18:47, Tejun Heo ha scritto: > > So, I'm still not convinced we need to go forward with full > > configurability. All use cases you described can be covered with > > per-class static filters + simple override switch to disable all, > > which would result in a lot simpler implementation w/ much smaller > > userland interface. > > I'm not sure the userland interface would be smaller, and it would be > more complex to get right: > > 1) how do you override the default? ioctl+SCM_RIGHTS or sysfs? Disabling filters if opened by root and tranfering via SCM_RIGHTS would be the simplest interface-wise (there's no new interface at all). Would that be too dangerous security-wise? > 2) do you need to override the default to "no access", "full access" and > "default access", or is a binary knob (default access/full access) > sufficient? Default / full should be enough, no? > 3) what capabilities control the setting? CAP_SYS_RAWIO seems to be a pretty good fit. > > What's the rationale for full configurability? > > Depending on the level of trust you have in your users, there are > different policies that are applicable. Even virtualization could have > a range of choices like "permit only standard operations", "also permit > UNMAP", "also permit persistent reservations", "permit everything > including vendor specific commands" I guess I just feel quite reluctant to expose another rather obscure userland configurable in-kernel filter and at the same time I'm not sure whether this is flexible enough. What if a device is shared by multiple virtual machines which are trusted at different levels? What if we end up actually having to filter cdb contents? I'm not trying to block it at all cost but let's make sure we looked into most possibilities before (re)adding this userland visible interface. Jens, James, what do you guys think? Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
(restoring cc lists) Hey, Paolo. On Thu, Oct 25, 2012 at 09:37:39AM +0200, Paolo Bonzini wrote: Il 24/10/2012 18:47, Tejun Heo ha scritto: So, I'm still not convinced we need to go forward with full configurability. All use cases you described can be covered with per-class static filters + simple override switch to disable all, which would result in a lot simpler implementation w/ much smaller userland interface. I'm not sure the userland interface would be smaller, and it would be more complex to get right: 1) how do you override the default? ioctl+SCM_RIGHTS or sysfs? Disabling filters if opened by root and tranfering via SCM_RIGHTS would be the simplest interface-wise (there's no new interface at all). Would that be too dangerous security-wise? 2) do you need to override the default to no access, full access and default access, or is a binary knob (default access/full access) sufficient? Default / full should be enough, no? 3) what capabilities control the setting? CAP_SYS_RAWIO seems to be a pretty good fit. What's the rationale for full configurability? Depending on the level of trust you have in your users, there are different policies that are applicable. Even virtualization could have a range of choices like permit only standard operations, also permit UNMAP, also permit persistent reservations, permit everything including vendor specific commands I guess I just feel quite reluctant to expose another rather obscure userland configurable in-kernel filter and at the same time I'm not sure whether this is flexible enough. What if a device is shared by multiple virtual machines which are trusted at different levels? What if we end up actually having to filter cdb contents? I'm not trying to block it at all cost but let's make sure we looked into most possibilities before (re)adding this userland visible interface. Jens, James, what do you guys think? Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: setting up CDB filters in udev (was Re: [PATCH v2 0/3] block: add queue-private command filter, editable via sysfs)
On Thu, Oct 25, 2012 at 09:37:39AM +0200, Paolo Bonzini wrote: Il 24/10/2012 18:47, Tejun Heo ha scritto: So, I'm still not convinced we need to go forward with full configurability. All use cases you described can be covered with per-class static filters + simple override switch to disable all, which would result in a lot simpler implementation w/ much smaller userland interface. I'm not sure the userland interface would be smaller, and it would be more complex to get right: 1) how do you override the default? ioctl+SCM_RIGHTS or sysfs? Disabling filters if opened by root and tranfering via SCM_RIGHTS would be the simplest interface-wise (there's no new interface at all). Would that be too dangerous security-wise? That would be a change with respect to what we have now. After transferring a root-opened (better: CAP_SYS_RAWIO-opened) file descriptor to an unprivileged process your SG_IO commands get filtered. So a ioctl is needed if you want to rely on SCM_RIGHTS. 2) do you need to override the default to no access, full access and default access, or is a binary knob (default access/full access) sufficient? Default / full should be enough, no? If a ioctl has to be added, I'd rather have at least none/full/default. 3) what capabilities control the setting? CAP_SYS_RAWIO seems to be a pretty good fit. Yes, for a ioctl it is (for sysfs CAP_SYS_ADMIN is better IMHO). I guess I just feel quite reluctant to expose another rather obscure userland configurable in-kernel filter and at the same time I'm not sure whether this is flexible enough. What if a device is shared by multiple virtual machines which are trusted at different levels? No, you just don't do that. If a device is passed through to virtual machines, it is between similar virtual machines (for some definition of similar). The only case where you have this sharing is in practice if either the device is read-only (my patch does give you a basic two-level filtering, with two separate bitmaps for RO and RW) or if you allow persistent reservations (which is as close to full trust as you can get). I'm not trying to block it at all cost but let's make sure we looked into most possibilities before (re)adding this userland visible interface. Sure, understood. :) Jens, James, what do you guys think? Paolo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
