lkml.org 
[lkml]   [2017]   [Jun]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 3.10 135/250] ocfs2: do not write error flag to user structure we cannot copy from/to
    Date
    From: Ben Hutchings <ben@decadent.org.uk>

    commit 2b462638e41ea62230297c21c4da9955937b7a3c upstream.

    If we failed to copy from the structure, writing back the flags leaks 31
    bits of kernel memory (the rest of the ir_flags field).

    In any case, if we cannot copy from/to the structure, why should we
    expect putting just the flags to work?

    Also make sure ocfs2_info_handle_freeinode() returns the right error
    code if the copy_to_user() fails.

    Fixes: ddee5cdb70e6 ('Ocfs2: Add new OCFS2_IOC_INFO ioctl for ocfs2 v8.')
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    Cc: Joel Becker <jlbec@evilplan.org>
    Acked-by: Mark Fasheh <mfasheh@suse.com>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
    Signed-off-by: Jiri Slaby <jslaby@suse.cz>
    Signed-off-by: Willy Tarreau <w@1wt.eu>
    ---
    fs/ocfs2/ioctl.c | 129 +++++++++++++++++++------------------------------------
    1 file changed, 43 insertions(+), 86 deletions(-)

    diff --git a/fs/ocfs2/ioctl.c b/fs/ocfs2/ioctl.c
    index 0c60ef2..b9d1609 100644
    --- a/fs/ocfs2/ioctl.c
    +++ b/fs/ocfs2/ioctl.c
    @@ -34,9 +34,8 @@
    copy_to_user((typeof(a) __user *)b, &(a), sizeof(a))

    /*
    - * This call is void because we are already reporting an error that may
    - * be -EFAULT. The error will be returned from the ioctl(2) call. It's
    - * just a best-effort to tell userspace that this request caused the error.
    + * This is just a best-effort to tell userspace that this request
    + * caused the error.
    */
    static inline void o2info_set_request_error(struct ocfs2_info_request *kreq,
    struct ocfs2_info_request __user *req)
    @@ -145,136 +144,105 @@ bail:
    int ocfs2_info_handle_blocksize(struct inode *inode,
    struct ocfs2_info_request __user *req)
    {
    - int status = -EFAULT;
    struct ocfs2_info_blocksize oib;

    if (o2info_from_user(oib, req))
    - goto bail;
    + return -EFAULT;

    oib.ib_blocksize = inode->i_sb->s_blocksize;

    o2info_set_request_filled(&oib.ib_req);

    if (o2info_to_user(oib, req))
    - goto bail;
    -
    - status = 0;
    -bail:
    - if (status)
    - o2info_set_request_error(&oib.ib_req, req);
    + return -EFAULT;

    - return status;
    + return 0;
    }

    int ocfs2_info_handle_clustersize(struct inode *inode,
    struct ocfs2_info_request __user *req)
    {
    - int status = -EFAULT;
    struct ocfs2_info_clustersize oic;
    struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);

    if (o2info_from_user(oic, req))
    - goto bail;
    + return -EFAULT;

    oic.ic_clustersize = osb->s_clustersize;

    o2info_set_request_filled(&oic.ic_req);

    if (o2info_to_user(oic, req))
    - goto bail;
    -
    - status = 0;
    -bail:
    - if (status)
    - o2info_set_request_error(&oic.ic_req, req);
    + return -EFAULT;

    - return status;
    + return 0;
    }

    int ocfs2_info_handle_maxslots(struct inode *inode,
    struct ocfs2_info_request __user *req)
    {
    - int status = -EFAULT;
    struct ocfs2_info_maxslots oim;
    struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);

    if (o2info_from_user(oim, req))
    - goto bail;
    + return -EFAULT;

    oim.im_max_slots = osb->max_slots;

    o2info_set_request_filled(&oim.im_req);

    if (o2info_to_user(oim, req))
    - goto bail;
    + return -EFAULT;

    - status = 0;
    -bail:
    - if (status)
    - o2info_set_request_error(&oim.im_req, req);
    -
    - return status;
    + return 0;
    }

    int ocfs2_info_handle_label(struct inode *inode,
    struct ocfs2_info_request __user *req)
    {
    - int status = -EFAULT;
    struct ocfs2_info_label oil;
    struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);

    if (o2info_from_user(oil, req))
    - goto bail;
    + return -EFAULT;

    memcpy(oil.il_label, osb->vol_label, OCFS2_MAX_VOL_LABEL_LEN);

    o2info_set_request_filled(&oil.il_req);

    if (o2info_to_user(oil, req))
    - goto bail;
    + return -EFAULT;

    - status = 0;
    -bail:
    - if (status)
    - o2info_set_request_error(&oil.il_req, req);
    -
    - return status;
    + return 0;
    }

    int ocfs2_info_handle_uuid(struct inode *inode,
    struct ocfs2_info_request __user *req)
    {
    - int status = -EFAULT;
    struct ocfs2_info_uuid oiu;
    struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);

    if (o2info_from_user(oiu, req))
    - goto bail;
    + return -EFAULT;

    memcpy(oiu.iu_uuid_str, osb->uuid_str, OCFS2_TEXT_UUID_LEN + 1);

    o2info_set_request_filled(&oiu.iu_req);

    if (o2info_to_user(oiu, req))
    - goto bail;
    -
    - status = 0;
    -bail:
    - if (status)
    - o2info_set_request_error(&oiu.iu_req, req);
    + return -EFAULT;

    - return status;
    + return 0;
    }

    int ocfs2_info_handle_fs_features(struct inode *inode,
    struct ocfs2_info_request __user *req)
    {
    - int status = -EFAULT;
    struct ocfs2_info_fs_features oif;
    struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);

    if (o2info_from_user(oif, req))
    - goto bail;
    + return -EFAULT;

    oif.if_compat_features = osb->s_feature_compat;
    oif.if_incompat_features = osb->s_feature_incompat;
    @@ -283,39 +251,28 @@ int ocfs2_info_handle_fs_features(struct inode *inode,
    o2info_set_request_filled(&oif.if_req);

    if (o2info_to_user(oif, req))
    - goto bail;
    + return -EFAULT;

    - status = 0;
    -bail:
    - if (status)
    - o2info_set_request_error(&oif.if_req, req);
    -
    - return status;
    + return 0;
    }

    int ocfs2_info_handle_journal_size(struct inode *inode,
    struct ocfs2_info_request __user *req)
    {
    - int status = -EFAULT;
    struct ocfs2_info_journal_size oij;
    struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);

    if (o2info_from_user(oij, req))
    - goto bail;
    + return -EFAULT;

    oij.ij_journal_size = osb->journal->j_inode->i_size;

    o2info_set_request_filled(&oij.ij_req);

    if (o2info_to_user(oij, req))
    - goto bail;
    + return -EFAULT;

    - status = 0;
    -bail:
    - if (status)
    - o2info_set_request_error(&oij.ij_req, req);
    -
    - return status;
    + return 0;
    }

    int ocfs2_info_scan_inode_alloc(struct ocfs2_super *osb,
    @@ -371,7 +328,7 @@ int ocfs2_info_handle_freeinode(struct inode *inode,
    u32 i;
    u64 blkno = -1;
    char namebuf[40];
    - int status = -EFAULT, type = INODE_ALLOC_SYSTEM_INODE;
    + int status, type = INODE_ALLOC_SYSTEM_INODE;
    struct ocfs2_info_freeinode *oifi = NULL;
    struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
    struct inode *inode_alloc = NULL;
    @@ -383,8 +340,10 @@ int ocfs2_info_handle_freeinode(struct inode *inode,
    goto out_err;
    }

    - if (o2info_from_user(*oifi, req))
    - goto bail;
    + if (o2info_from_user(*oifi, req)) {
    + status = -EFAULT;
    + goto out_free;
    + }

    oifi->ifi_slotnum = osb->max_slots;

    @@ -421,14 +380,16 @@ int ocfs2_info_handle_freeinode(struct inode *inode,

    o2info_set_request_filled(&oifi->ifi_req);

    - if (o2info_to_user(*oifi, req))
    - goto bail;
    + if (o2info_to_user(*oifi, req)) {
    + status = -EFAULT;
    + goto out_free;
    + }

    status = 0;
    bail:
    if (status)
    o2info_set_request_error(&oifi->ifi_req, req);
    -
    +out_free:
    kfree(oifi);
    out_err:
    return status;
    @@ -655,7 +616,7 @@ int ocfs2_info_handle_freefrag(struct inode *inode,
    {
    u64 blkno = -1;
    char namebuf[40];
    - int status = -EFAULT, type = GLOBAL_BITMAP_SYSTEM_INODE;
    + int status, type = GLOBAL_BITMAP_SYSTEM_INODE;

    struct ocfs2_info_freefrag *oiff;
    struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
    @@ -668,8 +629,10 @@ int ocfs2_info_handle_freefrag(struct inode *inode,
    goto out_err;
    }

    - if (o2info_from_user(*oiff, req))
    - goto bail;
    + if (o2info_from_user(*oiff, req)) {
    + status = -EFAULT;
    + goto out_free;
    + }
    /*
    * chunksize from userspace should be power of 2.
    */
    @@ -708,14 +671,14 @@ int ocfs2_info_handle_freefrag(struct inode *inode,

    if (o2info_to_user(*oiff, req)) {
    status = -EFAULT;
    - goto bail;
    + goto out_free;
    }

    status = 0;
    bail:
    if (status)
    o2info_set_request_error(&oiff->iff_req, req);
    -
    +out_free:
    kfree(oiff);
    out_err:
    return status;
    @@ -724,23 +687,17 @@ out_err:
    int ocfs2_info_handle_unknown(struct inode *inode,
    struct ocfs2_info_request __user *req)
    {
    - int status = -EFAULT;
    struct ocfs2_info_request oir;

    if (o2info_from_user(oir, req))
    - goto bail;
    + return -EFAULT;

    o2info_clear_request_filled(&oir);

    if (o2info_to_user(oir, req))
    - goto bail;
    + return -EFAULT;

    - status = 0;
    -bail:
    - if (status)
    - o2info_set_request_error(&oir, req);
    -
    - return status;
    + return 0;
    }

    /*
    --
    2.8.0.rc2.1.gbe9624a
    \
     
     \ /
      Last update: 2017-06-12 01:15    [W:4.405 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site