Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Thu, 5 Apr 2018 15:13:13 +0200
Message-ID: <CAMuHMdWovmqqgrOvtA=cyineJigwOtv-tGwz_PBVRFrgtryumw@mail.gmail.com>
Subject: net_dim() may use uninitialized data
To:     Tal Gilboa <talgi@mellanox.com>
Cc:     netdev <netdev@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Tal,

With gcc-4.1.2:

    drivers/net/ethernet/broadcom/bcmsysport.c: In function =E2=80=98bcm_sy=
sport_poll=E2=80=99:
    include/linux/net_dim.h:354: warning: =E2=80=98curr_stats.ppms=E2=80=99=
 may be
used uninitialized in this function
    include/linux/net_dim.h:354: warning: =E2=80=98curr_stats.bpms=E2=80=99=
 may be
used uninitialized in this function
    include/linux/net_dim.h:354: warning: =E2=80=98curr_stats.epms=E2=80=99=
 may be
used uninitialized in this function

Indeed, ...

| static inline void net_dim_calc_stats(struct net_dim_sample *start,
|                                       struct net_dim_sample *end,
|                                       struct net_dim_stats *curr_stats)
| {
|         /* u32 holds up to 71 minutes, should be enough */
|         u32 delta_us =3D ktime_us_delta(end->time, start->time);
|         u32 npkts =3D BIT_GAP(BITS_PER_TYPE(u32), end->pkt_ctr, start->pk=
t_ctr);
|         u32 nbytes =3D BIT_GAP(BITS_PER_TYPE(u32), end->byte_ctr,
|                              start->byte_ctr);
|
|         if (!delta_us)
|                 return;

... if delta_us is zero, none of the below will be initialized ...

|         curr_stats->ppms =3D DIV_ROUND_UP(npkts * USEC_PER_MSEC, delta_us=
);
|         curr_stats->bpms =3D DIV_ROUND_UP(nbytes * USEC_PER_MSEC, delta_u=
s);
|         curr_stats->epms =3D DIV_ROUND_UP(NET_DIM_NEVENTS * USEC_PER_MSEC=
,
|                                         delta_us);
| }
|
| static inline void net_dim(struct net_dim *dim,
|                            struct net_dim_sample end_sample)
| {
|         struct net_dim_stats curr_stats;
|         u16 nevents;
|
|         switch (dim->state) {
|         case NET_DIM_MEASURE_IN_PROGRESS:
|                 nevents =3D BIT_GAP(BITS_PER_TYPE(u16),
|                                   end_sample.event_ctr,
|                                   dim->start_sample.event_ctr);
|                 if (nevents < NET_DIM_NEVENTS)
|                         break;
|                 net_dim_calc_stats(&dim->start_sample, &end_sample,
|                                    &curr_stats);

... in the output parameter curr_stats ...

|                 if (net_dim_decision(&curr_stats, dim)) {

... and net_dim_decision will make some funky decisions based on
uninitialized data.

What are the proper values to initialize curr_stats with?
Alternatively, perhaps the call to net_dim_decision() should be made
dependent on delta_us being non-zero?

|                         dim->state =3D NET_DIM_APPLY_NEW_PROFILE;
|                         schedule_work(&dim->work);
|                         break;
|                 }
|                 /* fall through */
|         case NET_DIM_START_MEASURE:
|                 dim->state =3D NET_DIM_MEASURE_IN_PROGRESS;
|                 break;
|         case NET_DIM_APPLY_NEW_PROFILE:
|                 break;
|         }
| }

Gr{oetje,eeting}s,

                        Geert

--=20
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k=
.org

In personal conversations with technical people, I call myself a hacker. Bu=
t
when I'm talking to journalists I just say "programmer" or something like t=
hat.
                                -- Linus Torvalds