On Saturday, September 2, 2017 8:51:01 AM CEST Joe Perches wrote:
> On Thu, 2017-08-31 at 09:40 -0700, Joe Perches wrote:
> > On Thu, 2017-08-31 at 17:44 +0200, Greg Kroah-Hartman wrote:
> > > 4.9-stable review patch. If anyone has any objections, please let me know.
> > >
> > > ------------------
> > >
> > > From: Jiri Slaby <jslaby@xxxxxxx>
> > >
> > > commit 6f17581788206444cbbcdbc107498f85e9765e3d upstream.
> > >
> > > gcc 7 complains:
> > > drivers/net/wireless/intersil/p54/fwio.c: In function 'p54_scan':
> > > drivers/net/wireless/intersil/p54/fwio.c:491:4: warning: 'memset' used with length equal to number of elements without multiplication by element size [-Wmemset-elt-size]
> > >
> > > Fix that by passing the correct size to memset.
> > >
> > > Signed-off-by: Jiri Slaby <jslaby@xxxxxxx>
> > > Cc: Christian Lamparter <chunkeey@xxxxxxxxxxxxxx>
> > > Cc: Kalle Valo <kvalo@xxxxxxxxxxxxxx>
> > > Acked-by: Christian Lamparter <chunkeey@xxxxxxxxxxxxxx>
> > > Signed-off-by: Kalle Valo <kvalo@xxxxxxxxxxxxxx>
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > >
> > > ---
> > > drivers/net/wireless/intersil/p54/fwio.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > --- a/drivers/net/wireless/intersil/p54/fwio.c
> > > +++ b/drivers/net/wireless/intersil/p54/fwio.c
> > > @@ -488,7 +488,7 @@ int p54_scan(struct p54_common *priv, u1
> > >
> > > entry += sizeof(__le16);
> > > chan->pa_points_per_curve = 8;
> > > - memset(chan->curve_data, 0, sizeof(*chan->curve_data));
> > > + memset(chan->curve_data, 0, sizeof(chan->curve_data));
> > > memcpy(chan->curve_data, entry,
> > > sizeof(struct p54_pa_curve_data_sample) *
> > > min((u8)8, curve_data->points_per_channel));
> > >
> >
> > Why is this change correct?
> >
> > curve_data is a pointer.
> >
> > This now clears the sizeof a pointer and not
> > the sizeof struct p54_cal_database
>
> So what happens here?
> This change seems clearly incorrect.
> For all stable versions.
hm?
Please, just look again at [0]:
| struct p54_scan_body *chan = &body->normal;
| struct pda_pa_curve_data *curve_data =
| (void *) priv->curve_data->data;
|
| entry += sizeof(__le16);
| chan->pa_points_per_curve = 8;
| memset(chan->curve_data, 0, sizeof(chan->curve_data));
| memcpy(chan->curve_data, entry,
| sizeof(struct p54_pa_curve_data_sample) *
| min((u8)8, curve_data->points_per_channel));
yes: "curve_data" is a pointer. But memset and memcpy are using
"chan->curve_data". chan is a pointer to a p54_scan_body struct.
The structure is defined in lmac.h:
|struct p54_pa_curve_data_sample {
| u8 rf_power;
| u8 pa_detector;
| u8 data_barker;
| u8 data_bpsk;
| u8 data_qpsk;
| u8 data_16qam;
| u8 data_64qam;
| u8 padding;
|} __packed;
|
|struct p54_scan_body {
| u8 pa_points_per_curve;
| u8 val_barker;
| u8 val_bpsk;
| u8 val_qpsk;
| u8 val_16qam;
| u8 val_64qam;
| struct p54_pa_curve_data_sample curve_data[8];
| u8 dup_bpsk;
| u8 dup_qpsk;
| u8 dup_16qam;
| u8 dup_64qam;
|} __packed;
p54_scan_body's curve_data is an array of eight
p54_pa_curve_data_sample, each with eight u8.
This means that chan->curve_data is 64 bytes in total.
If you are not convinced yet, please add:
BUILD_BUG_ON(sizeof(chan->curve_data) != 64);
next to the memset and compile the driver.
If this was all wrong, this would cause a build error, right?
Regards,
Christian
[0] <http://elixir.free-electrons.com/linux/v4.13-rc7/source/drivers/net/wireless/intersil/p54/fwio.c#L485>
[1] <http://elixir.free-electrons.com/linux/v4.13-rc7/source/drivers/net/wireless/intersil/p54/lmac.h#L351>