Change default group and mode of /dev/pci to be the same as /dev/mem.

[kib]

Summary

Read access to either config space or BAR of an arbitrary PCI device should be considered to provide the same rights as complete read access to the physical memory and even more.

Discussed with: se

NB. An alternative could be to add a sysctl to administratively enable/disable BAR mapping through /dev/pci, or drop BAR ioctl on /dev/pci and provide second device where the ioctl is functional, with root:kmem 0640 rights. But I believe that config read access to the Intel config devices in uncore is sensitive enough already.

[jhb]

I suspect this breaks plain 'pciconf -l' for normal users. Note that plain pciconf -l doesn't do direct config access, it uses an ioctl that just returns the vendor and device IDs. Things like pciconf -lc or pciconf -r that actually do raw config access use ioctls that require write access and thus are only permitted to root by default. What is the problem you are trying to solve? Do you want to make the ioctl to map a BAR root-only and it is currently permitted for normal users? If so, add that ioctl to the list of ioctls that require FWRITE.

Oh, you explicitly added it to the list that only require FREAD. I would remove it from that last so that you require a writable fd to map the bar. The existing ioctls permitted for only FREAD are all careful to only contain "safe" information and not allow arbitrary access (e.g. GETBAR just returns the values of a BAR register, but that doesn't permit you to read/write the memory and you can't change the register either)

[kib]

Remove PCIOCBARMMAP from the list of allowed ro ioctls.

[kib]

In D16580#352116, @jhb wrote:

I suspect this breaks plain 'pciconf -l' for normal users. Note that plain pciconf -l doesn't do direct config access, it uses an ioctl that just returns the vendor and device IDs. Things like pciconf -lc or pciconf -r that actually do raw config access use ioctls that require write access and thus are only permitted to root by default. What is the problem you are trying to solve? Do you want to make the ioctl to map a BAR root-only and it is currently permitted for normal users? If so, add that ioctl to the list of ioctls that require FWRITE.

Oh, you explicitly added it to the list that only require FREAD. I would remove it from that last so that you require a writable fd to map the bar. The existing ioctls permitted for only FREAD are all careful to only contain "safe" information and not allow arbitrary access (e.g. GETBAR just returns the values of a BAR register, but that doesn't permit you to read/write the memory and you can't change the register either)

So are you suggesting the updated patch instead ? I can see how would it improve things for bar mmap. If you agree, I will update the review title and summary.

[jhb]

Yes, I think this is consistent with the other PCI ioctls, sorry I missed that previously. Does this satisfy your concerns re: security or do you think the kmem group should have write access to /dev/pci (as a separate change)?

[kib]

In D16580#352172, @jhb wrote:

Yes, I think this is consistent with the other PCI ioctls, sorry I missed that previously. Does this satisfy your concerns re: security or do you think the kmem group should have write access to /dev/pci (as a separate change)?

Yes, I think it satisfies the concerns and now it is really hold access permissions for config space same as for the bar mapping. Thank you.