Il 22/05/2013 17:03, Theodore Ts'o ha scritto: > Paolo, > > I'll probably regret butting my head into this, but it might be > helpful if you talk about your particular use case which is driving > your desire to make these changes. Ted, thank you very much. I understand that my discussion with Tejun is leading nowhere, and any outside help can only improve the situation. I hope you won't regret it. :) > For example, what do you think the > SG_IO whitelist _should_ be used, and why should it be made more > general? What's the use case that is being impaired by the current > state of how sg_io whitelists are being handled? Thanks for writing the questions down. I hope you don't mind the verbosity; perhaps we can use the opportunity to rewind the discussion. First of all, I'll note that SG_IO and block-device-specific ioctls both have their place. My usecase for SG_IO is virtualization, where I need to pass information from the LUN to the virtual machine with as much fidelity as possible if I choose to virtualize at the SCSI level. In that case, I'll use SG_IO. If people are okay with virtualizing at a higher-level, I'll use ioctls(BLK*) or fallocate (it depends on whether my target is a block device or a file). It depends on the application, the user, the context,... But in general, a program that cares about things like sense data must use SG_IO, a program that only cares about high-level concepts will use the ioctl. Now, SG_IO whitelist started so that you could "burn CDs without being root". But that can be extended to other device types; the SG_IO whitelist provides a way to allow low-level operations on devices while ensuring: a) respect of file permissions and no need for special privileges; b) no disruption of the devices or the storage fabric (for disks, no disruption beyond what would be possible anyway with read/write or other ioctls). There are some non-disk devices that (like CD-ROMs) have their own set of commands and can only be accessed via /dev/sg, for example media changers. Tapes also have some functionality that is not accessible via /dev/st. It would be useful (for virt, but I'm sure there are other usecases) to make the common uses of these devices possible without privileges, just by granting the appropriate permissions to the uids who will operate on them. This is why the SG_IO whitelist should be widened for these devices. But even for block devices, it should be made more general because the limitations in the current list are not really justified, except as an artifact of how the whitelist developed (again, "burn CDs without being root"). I think that there's no reason to forbid unprivileged programs from doing with SG_IO what they could do with other ioctls. By extension, if it makes sense to add an unprivileged ioctl in the future (e.g. for atomic compare and write) it should also be available from now via SG_IO. > Secondly, when you are trying to get a security vulnerability fixed, > it's helpful if you give the precise nature of the problem, and what > the an attacker can do with it. I think you are worried that if an > attacker has read-only access, they can still send the UNMAP command > which may (since it is advisory) result in a block no longer > containing valid data, such that a read will return zero's or some > other undefined garbage. Yes? Yes. > Now consider that if this is a high-priority fix, it's important to > make the patch as small as possible, since distributions (like your > employer) may want to backport the patch to older kernels. And > distribution release engineers will appreciate things if the patch is > as small as possible, making the _minimum_ necessary changes to fix > said security exposure. Generally, a series of 14 patches is __not__ > the minimum necessary patch. It is not a high-priority fix, or Red Hat would have agreed on an embargo date with other distros, and done all the security stuff that they routinely do. Still, it is a fix that I would like to get in, if only because Red Hat's policy is to get things upstream as much as possible. In fact, I would have very much preferred to get things upstream first. Unfortunately, this was not really possible in this case; FWIW, the RHEL kernel already has something very similar to the first 4 patches in v1 of this series. The reason for posting it all together, it's that frankly getting patches into block/ is an absolute pain. I know it's not a good reason, and I have certainly compounded my own mistakes. But guys, you have a review problem if things are allowed to sit in the mailing list for two months without a single reply. It's really the first time (and I've been working on free software for a _long_ time, as both volunteer and employee, maintainer and contributor) that I felt the helpless because I was not in the frequent contributors "clique". I know that's just an impression and doesn't match reality, but emotions do count and they make it really hard for one to keep his temper. I deleted many f-words from my replies (I should have deleted more). In fact, the (low) level of this discussion is also a first for me. Maybe I didn't know myself well enough, because even I wouldn't have expected it. I'm likely more surprised of this than anyone else. > Finally, please consider that your attitude is not going to win > friends and influence people. I do listen to review feedback, but I also expect the other side to listen to me, ask me what is not clear, and possess some knowledge of the domain that he's reviewing patches for. All of which, quite frankly, I have not seen in this case. > I don't know if the capability to work > well with upstream developers (people which ***other*** Red Hat > engineers have had no problems work with, and which I can attest, > through personal experience, are very reasonable engineers who are > easy to work with), is something which is a part of your performance > review process. But if it isn't, it probably should be, It is, as it should be. Paolo -- 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
