lkml.org 
[lkml]   [2021]   [Jul]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 01/64] media: omap3isp: Extract struct group for memcpy() region
    Date
    In preparation for FORTIFY_SOURCE performing compile-time and run-time
    field bounds checking for memcpy(), memmove(), and memset(), avoid
    intentionally writing across neighboring fields. Wrap the target region
    in a common named structure. This additionally fixes a theoretical
    misalignment of the copy (since the size of "buf" changes between 64-bit
    and 32-bit, but this is likely never built for 64-bit).

    FWIW, I think this code is totally broken on 64-bit (which appears to
    not be a "real" build configuration): it would either always fail (with
    an uninitialized data->buf_size) or would cause corruption in userspace
    due to the copy_to_user() in the call path against an uninitialized
    data->buf value:

    omap3isp_stat_request_statistics_time32(...)
    struct omap3isp_stat_data data64;
    ...
    omap3isp_stat_request_statistics(stat, &data64);

    int omap3isp_stat_request_statistics(struct ispstat *stat,
    struct omap3isp_stat_data *data)
    ...
    buf = isp_stat_buf_get(stat, data);

    static struct ispstat_buffer *isp_stat_buf_get(struct ispstat *stat,
    struct omap3isp_stat_data *data)
    ...
    if (buf->buf_size > data->buf_size) {
    ...
    return ERR_PTR(-EINVAL);
    }
    ...
    rval = copy_to_user(data->buf,
    buf->virt_addr,
    buf->buf_size);

    Regardless, additionally initialize data64 to be zero-filled to avoid
    undefined behavior.

    Fixes: 378e3f81cb56 ("media: omap3isp: support 64-bit version of omap3isp_stat_data")
    Signed-off-by: Kees Cook <keescook@chromium.org>
    ---
    drivers/media/platform/omap3isp/ispstat.c | 5 +--
    include/uapi/linux/omap3isp.h | 44 +++++++++++++++++------
    2 files changed, 36 insertions(+), 13 deletions(-)

    diff --git a/drivers/media/platform/omap3isp/ispstat.c b/drivers/media/platform/omap3isp/ispstat.c
    index 5b9b57f4d9bf..ea8222fed38e 100644
    --- a/drivers/media/platform/omap3isp/ispstat.c
    +++ b/drivers/media/platform/omap3isp/ispstat.c
    @@ -512,7 +512,7 @@ int omap3isp_stat_request_statistics(struct ispstat *stat,
    int omap3isp_stat_request_statistics_time32(struct ispstat *stat,
    struct omap3isp_stat_data_time32 *data)
    {
    - struct omap3isp_stat_data data64;
    + struct omap3isp_stat_data data64 = { };
    int ret;

    ret = omap3isp_stat_request_statistics(stat, &data64);
    @@ -521,7 +521,8 @@ int omap3isp_stat_request_statistics_time32(struct ispstat *stat,

    data->ts.tv_sec = data64.ts.tv_sec;
    data->ts.tv_usec = data64.ts.tv_usec;
    - memcpy(&data->buf, &data64.buf, sizeof(*data) - sizeof(data->ts));
    + data->buf = (uintptr_t)data64.buf;
    + memcpy(&data->frame, &data64.buf, sizeof(data->frame));

    return 0;
    }
    diff --git a/include/uapi/linux/omap3isp.h b/include/uapi/linux/omap3isp.h
    index 87b55755f4ff..0a16af91621f 100644
    --- a/include/uapi/linux/omap3isp.h
    +++ b/include/uapi/linux/omap3isp.h
    @@ -159,13 +159,25 @@ struct omap3isp_h3a_aewb_config {
    };

    /**
    - * struct omap3isp_stat_data - Statistic data sent to or received from user
    - * @ts: Timestamp of returned framestats.
    - * @buf: Pointer to pass to user.
    + * struct omap3isp_stat_frame - Statistic data without timestamp nor pointer.
    + * @buf_size: Size of buffer.
    * @frame_number: Frame number of requested stats.
    * @cur_frame: Current frame number being processed.
    * @config_counter: Number of the configuration associated with the data.
    */
    +struct omap3isp_stat_frame {
    + __u32 buf_size;
    + __u16 frame_number;
    + __u16 cur_frame;
    + __u16 config_counter;
    +};
    +
    +/**
    + * struct omap3isp_stat_data - Statistic data sent to or received from user
    + * @ts: Timestamp of returned framestats.
    + * @buf: Pointer to pass to user.
    + * @frame: Statistic data for frame.
    + */
    struct omap3isp_stat_data {
    #ifdef __KERNEL__
    struct {
    @@ -176,10 +188,15 @@ struct omap3isp_stat_data {
    struct timeval ts;
    #endif
    void __user *buf;
    - __u32 buf_size;
    - __u16 frame_number;
    - __u16 cur_frame;
    - __u16 config_counter;
    + union {
    + struct {
    + __u32 buf_size;
    + __u16 frame_number;
    + __u16 cur_frame;
    + __u16 config_counter;
    + };
    + struct omap3isp_stat_frame frame;
    + };
    };

    #ifdef __KERNEL__
    @@ -189,10 +206,15 @@ struct omap3isp_stat_data_time32 {
    __s32 tv_usec;
    } ts;
    __u32 buf;
    - __u32 buf_size;
    - __u16 frame_number;
    - __u16 cur_frame;
    - __u16 config_counter;
    + union {
    + struct {
    + __u32 buf_size;
    + __u16 frame_number;
    + __u16 cur_frame;
    + __u16 config_counter;
    + };
    + struct omap3isp_stat_frame frame;
    + };
    };
    #endif

    --
    2.30.2
    \
     
     \ /
      Last update: 2021-07-27 23:02    [W:4.052 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site