lkml.org 
[lkml]   [2022]   [Jan]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2 10/35] brcmfmac: firmware: Allow platform to override macaddr
    On Wed, Jan 5, 2022 at 3:26 PM Hector Martin <marcan@marcan.st> wrote:
    > On 04/01/2022 23.23, Andy Shevchenko wrote:
    > > On Tue, Jan 4, 2022 at 9:29 AM Hector Martin <marcan@marcan.st> wrote:

    ...

    > >> +#define BRCMF_FW_MACADDR_FMT "macaddr=%pM"

    > >> + snprintf(&nvp->nvram[nvp->nvram_len], BRCMF_FW_MACADDR_LEN + 1,
    > >> + BRCMF_FW_MACADDR_FMT, mac);
    > >
    > > Please, avoid using implict format string, it's dangerous from security p.o.v.
    >
    > What do you mean by implicit format string?

    When I read the above code I feel uncomfortable because no-one can see
    (without additional action and more reading and checking) if it's
    correct or not. This is potential to be error prone.

    > The format string is at the
    > top of the file and its length is right next to it, which makes it
    > harder for them to accidentally fall out of sync.

    It is not an argument. Just you may do the same in the code directly
    and more explicitly:

    Also you don't check the return code of snprintf which means that you
    don't care about the result, which seems to me wrong approach. If you
    don't care about the result, so it means it's not very important,
    right?

    > +#define BRCMF_FW_MACADDR_FMT "macaddr=%pM"
    > +#define BRCMF_FW_MACADDR_LEN (7 + ETH_ALEN * 3)



    --
    With Best Regards,
    Andy Shevchenko

    \
     
     \ /
      Last update: 2022-01-06 15:21    [W:3.835 / U:0.808 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site