Messages in this thread | | | From | Michael Buesch <> | Subject | Re: [patch 2/6] New Generic HW RNG | Date | Sun, 7 May 2006 15:16:08 +0200 |
| |
On Sunday 07 May 2006 15:03, you wrote: > This does not handle the case of partial read correctly - the code > should be > > return ret ? : -ERESTARTSYS; > > > + if (!current_rng) { > > + mutex_unlock(&rng_mutex); > > + return -ENODEV; > > The same problem here (although finding the RNG suddenly missing after > we heve just read something from it is pretty unlikely). > > > + } > > + have_data = 0; > > + if (current_rng->data_present == NULL || > > + current_rng->data_present(current_rng)) > > + have_data = current_rng->data_read(current_rng, &data); > > + mutex_unlock(&rng_mutex); > > + > > + while (have_data && size) { > > + if (put_user((u8)data, buf++)) { > > + ret = ret ? : -EFAULT; > > + break; > > + } > > + size--; > > + ret++; > > + have_data--; > > + data>>=8; > > + } > > + > > + if (filp->f_flags & O_NONBLOCK) > > + return ret ? : -EAGAIN; > > + > > + if (need_resched()) { > > + schedule_timeout_interruptible(1); > > + } else { > > + err = mutex_lock_interruptible(&rng_mutex); > > + if (err) > > + return err; > > And here... > > > + if (!current_rng) { > > + mutex_unlock(&rng_mutex); > > + return -ENODEV; > > And here too.
Whoops, will fix these.
> > + list_for_each_entry(rng, &rng_list, list) { > > + if (strncmp(rng->name, buf, len) == 0) { > > This will match if the passed string is just a prefix of rng->name. > Apparently sysfs guarantees that the buffer passed to ->store will be > NUL-terminated, so this should be just a strcmp().
I am not sure if it is guaranteed NUL terminated. Greg? But I will look into this.
> > + if (rng->init) { > > + err = rng->init(rng); > > + if (err) > > + break; > > + } > > + if (current_rng && current_rng->cleanup) > > + current_rng->cleanup(current_rng); > > What if rng == current_rng here (someone has written the same RNG name > to the "store" attribute)? The lowlevel RNG driver should not have to > handle nested init/cleanup calls.
I see. Will fix this.
I will also fix the bcm43xx patch. It registers always with the same "name". That will blow up, if there is more than one bcm43xx device in the system.
-- Greetings Michael. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |