lkml.org 
[lkml]   [2001]   [Oct]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] random.c bugfix
    On Oct 29, 2001  18:23 -0600, Oliver Xymoron wrote:
    > On Mon, 29 Oct 2001, Andreas Dilger wrote:
    > > Having an accumulator would only handle a rarely-used corner case. We
    > > could just as easily fix any user-space entropy daemon to write at least
    > > 4 bytes at a time. Alternately, we could "pad" with enough bytes from
    > > the random pool, and not accumulate at all.
    >
    > Padding -from the pool- is acceptable (and simple enough a slow path to
    > add to the low-level function). Padding with constants is bad and padding
    > with zeros tends to be really bad.

    I would rather fix this at the caller and not in the low-level code, because
    a large majority of the callers (esp. on the fast path) only use full word
    inputs. Even for the random_write() case, we would only need this at most
    a single time per write, instead of once per "hunk" (=64 bytes). What I
    have done is to accumulate bytes in random_write() until we get a full word.

    > > In any case, this is in the noise compared to not using the input data
    > > at all (which I fixed in the other patch).
    >
    > Any of this made it into recent kernels yet? Backport to 2.2 might be a
    > good idea too..

    Well, I just saw that the "in++" and "nwords * 4" patches went into -pre4.
    These are the only real non-cosmetic parts of what has been sent. The
    other patches were not officially submitted to Linus yet (using bytes
    as parameters, and removing poolwords from the struct). I have reverted
    those patches in my tree, and gone back to using words as units for
    add_entropy(), since it doesn't make sense to take bytes as a parameter
    and then require a multiple of 4 bytes for input sizes.

    The following patch changes random_write() to accumulate partial words.
    It should apply against any recent kernel, and assumes the two "byte"
    patches I previously sent out are not applied. Haven't had a chance
    to test it yet, but it compiles.

    Cheers, Andreas
    ===========================================================================
    --- linux.orig/drivers/char/random.c Thu Oct 25 03:04:34 2001
    +++ linux/drivers/char/random.c Mon Oct 29 20:31:42 2001
    @@ -1576,34 +1576,56 @@
    }

    static ssize_t
    -random_write(struct file * file, const char * buffer,
    - size_t count, loff_t *ppos)
    +random_write(struct file *file, const char *buffer, size_t count, loff_t *ppos)
    {
    + static char save[4];
    + static int nsave = 0;
    int ret = 0;
    - size_t bytes;
    - __u32 buf[16];
    - const char *p = buffer;
    - size_t c = count;
    + const __u32 *p = (__u32 *)buffer;
    + size_t w = count / 4;
    + int tail = count & 3;
    + int extra = 0;
    +
    + while (w > 0) {
    + size_t words;
    + __u32 buf[16];

    - while (c > 0) {
    - bytes = MIN(c, sizeof(buf));
    + words = min(w, sizeof(buf) / 4);

    - bytes -= copy_from_user(&buf, p, bytes);
    - if (!bytes) {
    + if (copy_from_user(buf, p, words * 4)) {
    ret = -EFAULT;
    - break;
    + goto out;
    }
    - c -= bytes;
    - p += bytes;
    + w -= words;
    + p += words;

    - add_entropy_words(random_state, buf, (bytes + 3) / 4);
    + add_entropy_words(random_state, p, words);
    + }
    +
    + /* Accumulate partial words until we get a full word for addition */
    + while (tail) {
    + int bytes = min(tail, 4 - nsave);
    +
    + if (copy_from_user(save + nsave, (char *)p + extra, bytes)) {
    + ret = -EFAULT;
    + goto out;
    + }
    + nsave += bytes;
    + extra += bytes;
    + tail -= bytes;
    +
    + if (nsave >= 4) {
    + add_entropy_words(random_state, (__u32 *)save, 1);
    + nsave = 0;
    + }
    }
    - if (p == buffer) {
    +out:
    + if (p == (__u32 *)buffer && extra == 0) {
    return (ssize_t)ret;
    } else {
    file->f_dentry->d_inode->i_mtime = CURRENT_TIME;
    mark_inode_dirty(file->f_dentry->d_inode);
    - return (ssize_t)(p - buffer);
    + return (ssize_t)((p - (__u32 *)buffer) * 4 + extra);
    }
    }

    --
    Andreas Dilger
    http://sourceforge.net/projects/ext2resize/
    http://www-mddsp.enel.ucalgary.ca/People/adilger/
    -
    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/

    \
     
     \ /
      Last update: 2005-03-22 13:12    [W:17.933 / U:0.548 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site