Message ID | 1499078279-19135-1-git-send-email-freude@linux.vnet.ibm.com (mailing list archive) |
---|---|
State | Rejected |
Delegated to: | Herbert Xu |
Headers | show |
On Mon, Jul 03, 2017 at 12:37:59PM +0200, Harald Freudenberger wrote: > Currently /dev/hwrng uses default device node permissions > which is 0600. So by default the device node is not accessible > by an ordinary user. Some distros do rewrite the device node > permissions via udev rule, others don't. This patch provides > 0444 as the new mode value and so makes the device node > accessible for all users without the need to have udev rules > rewriting the access rights. > > Signed-off-by: Harald Freudenberger <freude@linux.vnet.ibm.com> Hmm, one usage scenario for /dev/hwrng is to feed rngd which then feeds into /dev/random. In that case it may not be desirable to allow arbitrary access to hwrgn since it may cause the rate of entropy going into /dev/random to go down. In any case, as you noted userspace can change this anyway so I don't see why we need to make this policy change in the kernel. Cheers,
On 07/12/2017 12:13 PM, Herbert Xu wrote: > On Mon, Jul 03, 2017 at 12:37:59PM +0200, Harald Freudenberger wrote: >> Currently /dev/hwrng uses default device node permissions >> which is 0600. So by default the device node is not accessible >> by an ordinary user. Some distros do rewrite the device node >> permissions via udev rule, others don't. This patch provides >> 0444 as the new mode value and so makes the device node >> accessible for all users without the need to have udev rules >> rewriting the access rights. >> >> Signed-off-by: Harald Freudenberger <freude@linux.vnet.ibm.com> > Hmm, one usage scenario for /dev/hwrng is to feed rngd which then > feeds into /dev/random. In that case it may not be desirable to > allow arbitrary access to hwrgn since it may cause the rate of > entropy going into /dev/random to go down. > > In any case, as you noted userspace can change this anyway so I > don't see why we need to make this policy change in the kernel. > > Cheers, It was worth a try to get rid of complains from customers. However, your argument about the possible weakness in the entropy pool for /dev/random with pumping hwrng dry does not really fit: This can be easier done by just pulling random directly from /dev/random, as every distro I could get a hand on uses crw-rw-rw- permissions on /dev/random and /dev/urandom. Thanks Harald Freudenberger
Hi Herbert, On 12 July 2017 at 15:43, Herbert Xu <herbert@gondor.apana.org.au> wrote: > Hmm, one usage scenario for /dev/hwrng is to feed rngd which then > feeds into /dev/random. In that case it may not be desirable to > allow arbitrary access to hwrgn since it may cause the rate of > entropy going into /dev/random to go down. > > In any case, as you noted userspace can change this anyway so I > don't see why we need to make this policy change in the kernel. Looking at the comment in https://android.googlesource.com/platform/frameworks/base/+/master/services/core/java/com/android/server/EntropyMixer.java#145 I am wondering whether your concern is a problem. I do not know whether the comment in Android source is valid so please ignore my ignorance. Regards, PrasannaKumar
diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c index 503a41d..4093db5 100644 --- a/drivers/char/hw_random/core.c +++ b/drivers/char/hw_random/core.c @@ -285,6 +285,7 @@ static struct miscdevice rng_miscdev = { .minor = HWRNG_MINOR, .name = RNG_MODULE_NAME, .nodename = "hwrng", + .mode = 0444, .fops = &rng_chrdev_ops, .groups = rng_dev_groups, };
Currently /dev/hwrng uses default device node permissions which is 0600. So by default the device node is not accessible by an ordinary user. Some distros do rewrite the device node permissions via udev rule, others don't. This patch provides 0444 as the new mode value and so makes the device node accessible for all users without the need to have udev rules rewriting the access rights. Signed-off-by: Harald Freudenberger <freude@linux.vnet.ibm.com> --- drivers/char/hw_random/core.c | 1 + 1 file changed, 1 insertion(+)