lkml.org 
[lkml]   [2004]   [Feb]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH/proposal] dm-crypt: add digest-based iv generation mode
    From
    Date
    Am Do, den 26.02.2004 schrieb Matt Mackall um 21:02:

    > User is giving us the size of his buffer, not the size of the tfm
    > which we already know. We refuse to copy if buffer is not big enough,
    > otherwise return number of bytes copied.

    Well, I would usually except the user knows what he does, but okay, if
    you think that's safer. It requires the user to carry the size of the
    buffer around. Assuming he kmallocs the buffer in one function with the
    correct size and wants to use it in another function (a mempool or
    something, who knows). He doesn't know the size of the buffer there.

    > This may seem a little
    > redundant for the on-stack usage of the API, but may make sense in
    > other cases.

    It may, yes. But I don't think this kind of thing is done elsewhere in
    the kernel. It's okay for things like user space libraries where the
    libraries and users can be compiled separately to catch problems with
    ABI changes, but in the kernel? I think it's overkill.

    These things should be caught using BUG_ONs if you thing someone might
    get them wrong somehow und in the future if something changes. But now
    if they add additional parameters.

    > > > +void crypto_cleanup_copy_tfm(char *user_tfm)
    > > > +{
    > > > + crypto_exit_ops((struct crypto_tfm *)user_tfm);
    > >
    > > This looks dangerous. The algorithm might free a buffer. This is only
    > > safe if we introduce per-algorithm copy methods that also duplicate
    > > external buffers.
    >
    > I'm currently working under the assumption that such external buffers
    > are unnecessary but I haven't done the audit. If and when such code
    > exist, such code should be added, yes. Hence the comment in the copy
    > function:
    >
    > + /* currently assumes shallow copy is sufficient */

    Ok, I see.

    We could add some functions so that everything is symmetric:
    (the names with a star are already existing)

    *crypto_alloc_tfm
    \_ crypto_init_tfm

    *crypto_free_tfm
    \_ crypto_release_tfm

    crypto_clone_tfm
    \_ crypto_copy_tfm

    crypto_get_alg_size
    \_crypto_get_tfm_size

    crypto_init_tfm does everything but the kmalloc
    crypto_release_tfm everything but the kfree

    So crypto_alloc_tfm and crypto_release_tfm can be changed to call
    crypto_init_fdm and crypto_release_tfm plus crypto_get_alg_size/kmalloc
    and kfree.

    crypto_clone_tfm calls crypto_get_tfm_size, kmalloc and crypto_copy_tfm.
    crypto_copy_tfm copies the tfm structure, cares about algorithm
    reference counting and calls a (new) copy method. This copy method
    should copy things in its context like kmalloc'ed structures (or
    increment a reference count if it's a static memory structure or
    something). (however kmalloc's should be avoided if possible, the
    variable sized context provides some flexibility)

    crypto_get_alg_size returns the size of the tfm structure when algorithm
    name and flags are given, crypto_get_tfm size returns the size of an
    existing tfm structure.

    So we can also directly initialize a tfm structure on a stack, not only
    copy it to a stack. Very flexible.

    What do you think?


    -
    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 14:01    [W:2.620 / U:0.548 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site