Michael Buesch <[email protected]> wrote:
>
> ...
>
> +static inline
> +int hwrng_init(struct hwrng *rng)
> +static inline
> +void hwrng_cleanup(struct hwrng *rng)
> +static inline
> +int hwrng_data_present(struct hwrng *rng)
> +static inline
> +int hwrng_data_read(struct hwrng *rng, u32 *data)
Lose the newlines, please.
> +static int rng_dev_open(struct inode *inode, struct file *filp)
Like that.
> +static ssize_t rng_dev_read(struct file *filp, char __user *buf,
> + size_t size, loff_t *offp)
> +{
> + int have_data;
> + u32 data;
> + ssize_t ret = 0;
> + int i, err = 0;
> +
> + while (size) {
> + err = -ERESTARTSYS;
> + if (mutex_lock_interruptible(&rng_mutex))
> + goto out;
> + if (!current_rng) {
> + mutex_unlock(&rng_mutex);
> + err = -ENODEV;
> + goto out;
> + }
> + have_data = 0;
> + if (hwrng_data_present(current_rng))
> + have_data = hwrng_data_read(current_rng, &data);
> + mutex_unlock(&rng_mutex);
> +
> + err = -EFAULT;
> + while (have_data && size) {
> + if (put_user((u8)data, buf++))
> + goto out;
> + size--;
> + ret++;
> + have_data--;
> + data >>= 8;
> + }
> +
> + err = -EAGAIN;
> + if (filp->f_flags & O_NONBLOCK)
> + goto out;
> +
> + err = -ERESTARTSYS;
> + if (need_resched()) {
> + if (schedule_timeout_interruptible(1))
> + goto out;
> + } else {
> + if (mutex_lock_interruptible(&rng_mutex))
> + goto out;
> + if (!current_rng) {
> + mutex_unlock(&rng_mutex);
> + err = -ENODEV;
> + goto out;
> + }
> + for (i = 0; i < 20; i++) {
> + if (hwrng_data_present(current_rng))
> + break;
> + udelay(10);
> + }
> + mutex_unlock(&rng_mutex);
> + }
> + if (signal_pending(current))
> + goto out;
> + }
> +out:
> + return ret ? : err;
> +}
What's going on with the need_resched() tricks in there? (Unobvious, needs
a comment). From my reading, it'll cause a caller to this function to hang
for arbitrary periods of time if something if causing heavy scheduling
pressure.
What's the polling of hwrng_data_present() doing in here? (Unobvious,
needs a comment).
> +static ssize_t hwrng_attr_current_store(struct class_device *class,
> + const char *buf, size_t len)
> +{
> + int err;
> + struct hwrng *rng;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
Are the sysfs permissions not adequate?
> +MODULE_AUTHOR("The Linux Kernel team");
Mutter. Might as well remove this.
A MAINTAINERS record would be nice.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
