netdev - Re: [PATCH v3] net: bnxt_ptp: fix compilation error

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACKFLi=wzz5GrBUgc1dCNGUuKs0HdrVo_m4LonJF-9sO3kkf5Q@mail.gmail.com>
Date:   Mon, 28 Mar 2022 01:26:28 -0700
From:   Michael Chan <michael.chan@...adcom.com>
To:     Damien Le Moal <damien.lemoal@...nsource.wdc.com>
Cc:     Pavan Chebbi <pavan.chebbi@...adcom.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Linux Netdev List <netdev@...r.kernel.org>
Subject: Re: [PATCH v3] net: bnxt_ptp: fix compilation error

On Mon, Mar 28, 2022 at 12:01 AM Damien Le Moal
<damien.lemoal@...nsource.wdc.com> wrote:
>
> On 3/28/22 15:38, Pavan Chebbi wrote:
> > On Mon, Mar 28, 2022 at 11:57 AM Damien Le Moal
> > <damien.lemoal@...nsource.wdc.com> wrote:
> >>
> >> The Broadcom bnxt_ptp driver does not compile with GCC 11.2.2 when
> >> CONFIG_WERROR is enabled. The following error is generated:
> >>
> >> drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c: In function ‘bnxt_ptp_enable’:
> >> drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c:400:43: error: array
> >> subscript 255 is above array bounds of ‘struct pps_pin[4]’
> >> [-Werror=array-bounds]
> >>   400 |  ptp->pps_info.pins[pin_id].event = BNXT_PPS_EVENT_EXTERNAL;
> >>       |  ~~~~~~~~~~~~~~~~~~^~~~~~~~
> >> In file included from drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c:20:
> >> drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h:75:24: note: while
> >> referencing ‘pins’
> >>    75 |         struct pps_pin pins[BNXT_MAX_TSIO_PINS];
> >>       |                        ^~~~
> >> cc1: all warnings being treated as errors
> >>
> >> This is due to the function ptp_find_pin() returning a pin ID of -1 when
> >> a valid pin is not found and this error never being checked.
> >> Change the TSIO_PIN_VALID() function to also check that a pin ID is not
> >> negative and use this macro in bnxt_ptp_enable() to check the result of
> >> the calls to ptp_find_pin() to return an error early for invalid pins.
> >> This fixes the compilation error.
> >>
> >> Fixes: 9e518f25802c ("bnxt_en: 1PPS functions to configure TSIO pins")
> >> Cc: <stable@...r.kernel.org>
> >> Signed-off-by: Damien Le Moal <damien.lemoal@...nsource.wdc.com>
> >> ---
> >> Changes from v2:
> >> * Restore the improved check in TSIO_PIN_VALID() and use this macro to
> >>   return an error early in bnxt_ptp_enable() in case of invalid pin ID.
> >> Changes from v1:
> >> * No need to change the TSIO_PIN_VALID() macro as pin_id is an unsigned
> >>   value.
> >>
> >>  drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 6 +++++-
> >>  drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h | 2 +-
> >>  2 files changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> >> index a0b321a19361..9c2ad5e67a5d 100644
> >> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> >> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> >> @@ -382,7 +382,7 @@ static int bnxt_ptp_enable(struct ptp_clock_info *ptp_info,
> >>         struct bnxt_ptp_cfg *ptp = container_of(ptp_info, struct bnxt_ptp_cfg,
> >>                                                 ptp_info);
> >>         struct bnxt *bp = ptp->bp;
> >> -       u8 pin_id;
> >> +       int pin_id;
> >>         int rc;
> >>
> >>         switch (rq->type) {
> >> @@ -390,6 +390,8 @@ static int bnxt_ptp_enable(struct ptp_clock_info *ptp_info,
> >>                 /* Configure an External PPS IN */
> >>                 pin_id = ptp_find_pin(ptp->ptp_clock, PTP_PF_EXTTS,
> >>                                       rq->extts.index);
> >> +               if (!TSIO_PIN_VALID(pin_id))
> >> +                       return -EOPNOTSUPP;
> >
> > Thanks. Could we now remove this check from the function bnxt_ptp_cfg_pin() ?
>
> Having a quick glance at all the call sites, it looks like it would be OK.
> But may be in a different patch ?
>

Yes, we can improve it later in a separate patch.  Thanks.

Reviewed-by: Michael Chan <michael.chan@...adcom.com>

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4209 bytes)