2008/6/26 Jens Axboe <jens.axboe@xxxxxxxxxx>: > On Thu, Jun 26 2008, Adel Gadllah wrote: >> 2008/6/26 FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx>: >> > On Fri, 27 Jun 2008 00:08:46 +0900 >> > FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx> wrote: >> > >> >> On Thu, 26 Jun 2008 17:05:50 +0200 >> >> "Adel Gadllah" <adel.gadllah@xxxxxxxxx> wrote: >> >> >> >> > 2008/6/26 FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx>: >> >> > > On Thu, 26 Jun 2008 12:10:25 +0200 >> >> > > "Adel Gadllah" <adel.gadllah@xxxxxxxxx> wrote: >> >> > > >> >> > >> 2008/6/18 Peter Jones <pjones@xxxxxxxxxx>: >> >> > >> > Douglas Gilbert wrote: >> >> > >> >> >> >> > >> >> Peter Jones wrote: >> >> > >> >>> >> >> > >> >>> FUJITA Tomonori wrote: >> >> > >> >>> >> >> > >> >>>> Well, this changes sg behaviour since sg's allow_ops filter has a >> >> > >> >>>> access permission different from blk_verify_command filter's. >> >> > >> >>> >> >> > >> >>> > >> >> > >> >>>> >> >> > >> >>>> I guess that the first thing you need to do is that figuring out a >> >> > >> >>>> proper access permission for each command, which sg maintainer, etc >> >> > >> >>>> can agree. It's pretty hard and that's the reason why this patch has >> >> > >> >>>> not been merged for years, I think. >> >> > >> >>> >> >> > >> >>> I don't think this logic is sound. >> >> > >> >> >> >> > >> >> That depends on your viewpoint. >> >> > >> > >> >> > >> > My viewpoint is this: >> >> > >> > >> >> > >> > 1) Whether you agree with his reasons or not, Linus made it pretty clear >> >> > >> > that he's against removing the command filter (see >> >> > >> > http://marc.info/?l=linux-scsi&m=115419945212450&w=2 ) >> >> > >> > 2) Having different code paths use different filtering code just adds more >> >> > >> > confusion. >> >> > >> > 3) If we're going to have filtering, it should be configurable on a >> >> > >> > per-device basis from userland. >> >> > >> > >> >> > >> > Which of these do you disagree with? >> >> > >> > >> >> > >> > [...] >> >> > >> >> >> >> > >> >> Are per device command filters being proposed? >> >> > >> > >> >> > >> > Yes, that's what the patch implements. And it allows the userland to >> >> > >> > configure them according to the needs of the hardware. >> >> > >> >> >> > >> Jens can we add merge this for .27 or does anyone still has objections? >> >> > > >> >> > > I think that this patch makes sg's permission stricter. So this could >> >> > > break the existing user-space applications. >> >> > > >> >> > >> >> > any particular app in mind? >> >> >> >> No, but there would be some. >> >> >> >> >> >> > for write access it still allows all commands (because there are some >> >> > userspace apps tha rely on this). >> >> >> >> Yeah, I know. But for read access, some commands will be blocked. >> > >> > I think that it's not a good idea to say "this patch could break >> > something but we have no idea about them. So we can merge this." >> > >> > It's better to loosen scsi_ioctl's permissions to match with sg's >> > permission. >> >> agreed. >> >> Jens please apply the attached patch on top of the old one. > > Thanks, I thought this had already been worked out when I saw the > previous discussion. Please verify that this patch then no longer causes > ANY different behaviour than what we already have, that's a prerequisite > for merging it. With this patch all apps that worked with the old scsi_ioctl, bsg and sg filter should behave exactly the same as without the new filter infrastructure. (it adds no new restrictions) The only thing it adds is that it allows READ_CAPACITY, REPORT_LUNS, SERVICE_ACTION_IN, RECEIVE_DIAGNOSTIC and MAINTENANCE_IN_CMD for bsg and scsi_ioctl in read only mode. As this was already allowed using /dev/sg* the commands should be safe, so no regression added. > I've applied this patch. Thanks, Adel -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
