lkml.org 
[lkml]   [2021]   [Jul]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 42/64] net: qede: Use memset_after() for counters
    On Tue, Jul 27, 2021 at 01:58:33PM -0700, Kees Cook wrote:
    > In preparation for FORTIFY_SOURCE performing compile-time and run-time
    > field bounds checking for memset(), avoid intentionally writing across
    > neighboring fields.
    >
    > Use memset_after() so memset() doesn't get confused about writing
    > beyond the destination member that is intended to be the starting point
    > of zeroing through the end of the struct.
    >
    > Signed-off-by: Kees Cook <keescook@chromium.org>
    > ---
    > The old code seems to be doing the wrong thing: starting from not the
    > first member, but sized for the whole struct. Which is correct?

    Quick ping on this question.

    The old code seems to be doing the wrong thing: it starts from the second
    member and writes beyond int_info, clobbering qede_lock:

    struct qede_dev {
    ...
    struct qed_int_info int_info;

    /* Smaller private variant of the RTNL lock */
    struct mutex qede_lock;
    ...


    struct qed_int_info {
    struct msix_entry *msix;
    u8 msix_cnt;

    /* This should be updated by the protocol driver */
    u8 used_cnt;
    };

    Should this also clear the "msix" member, or should this not write
    beyond int_info? This patch does the latter.

    -Kees

    > ---
    > drivers/net/ethernet/qlogic/qede/qede_main.c | 2 +-
    > 1 file changed, 1 insertion(+), 1 deletion(-)
    >
    > diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
    > index 01ac1e93d27a..309dfe8c94fb 100644
    > --- a/drivers/net/ethernet/qlogic/qede/qede_main.c
    > +++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
    > @@ -2419,7 +2419,7 @@ static int qede_load(struct qede_dev *edev, enum qede_load_mode mode,
    > goto out;
    > err4:
    > qede_sync_free_irqs(edev);
    > - memset(&edev->int_info.msix_cnt, 0, sizeof(struct qed_int_info));
    > + memset_after(&edev->int_info, 0, msix);
    > err3:
    > qede_napi_disable_remove(edev);
    > err2:
    > --
    > 2.30.2
    >

    --
    Kees Cook

    \
     
     \ /
      Last update: 2021-07-31 18:08    [W:4.143 / U:1.204 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site