| Message ID | 20240619-garmin_gps_counted_by-v1-1-d8d816f085d9@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | USB: serial: garmin_gps: annotate struct garmin_packet with __counted_by | expand |
On Wed, Jun 19, 2024 at 11:40:57AM +0200, Javier Carrasco wrote: > Use the __counted_by compiler attribute for the data[] flexible array member > to improve the results of array bound sanitizers. > > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> > --- > The size is assigned before the first reference to the flexible array > (see pkt_add()), which allows for a straightforward annotation without > further modifications. Agreed, this seems like a reasonable patch in and of itself that should work: Reviewed-by: Nathan Chancellor <nathan@kernel.org> It might also make sense to change the pkt allocation to use struct_size() instead of open coding it? diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c index 670e942fdaaa..04946c1cbf6f 100644 --- a/drivers/usb/serial/garmin_gps.c +++ b/drivers/usb/serial/garmin_gps.c @@ -267,8 +267,7 @@ static int pkt_add(struct garmin_data *garmin_data_p, /* process only packets containing data ... */ if (data_length) { - pkt = kmalloc(sizeof(struct garmin_packet)+data_length, - GFP_ATOMIC); + pkt = kmalloc(struct_size(pkt, data, data_length), GFP_ATOMIC); if (!pkt) return 0; > --- > drivers/usb/serial/garmin_gps.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c > index 670e942fdaaa..57df6ad183ff 100644 > --- a/drivers/usb/serial/garmin_gps.c > +++ b/drivers/usb/serial/garmin_gps.c > @@ -104,7 +104,7 @@ struct garmin_packet { > int seq; > /* the real size of the data array, always > 0 */ > int size; > - __u8 data[]; > + __u8 data[] __counted_by(size); > }; > > /* structure used to keep the current state of the driver */ > > --- > base-commit: 0c52056d9f77508cb6d4d68d3fc91c6c08ec71af > change-id: 20240619-garmin_gps_counted_by-376545960353 > > Best regards, > -- > Javier Carrasco <javier.carrasco.cruz@gmail.com> >
On 19/06/24 16:43, Nathan Chancellor wrote: > On Wed, Jun 19, 2024 at 11:40:57AM +0200, Javier Carrasco wrote: >> Use the __counted_by compiler attribute for the data[] flexible array member >> to improve the results of array bound sanitizers. >> >> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> >> --- >> The size is assigned before the first reference to the flexible array >> (see pkt_add()), which allows for a straightforward annotation without >> further modifications. > > Agreed, this seems like a reasonable patch in and of itself that should > work: > > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > > It might also make sense to change the pkt allocation to use > struct_size() instead of open coding it? +1 :) Thanks -- Gustavo
On 19/06/2024 16:43, Nathan Chancellor wrote: > On Wed, Jun 19, 2024 at 11:40:57AM +0200, Javier Carrasco wrote: >> Use the __counted_by compiler attribute for the data[] flexible array member >> to improve the results of array bound sanitizers. >> >> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> >> --- >> The size is assigned before the first reference to the flexible array >> (see pkt_add()), which allows for a straightforward annotation without >> further modifications. > > Agreed, this seems like a reasonable patch in and of itself that should > work: > > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > > It might also make sense to change the pkt allocation to use > struct_size() instead of open coding it? Sure, it looks much better. I will send a v2 with the patch and the corresponding Suggested-by: Thanks and best regards, Javier Carrasco
diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c index 670e942fdaaa..57df6ad183ff 100644 --- a/drivers/usb/serial/garmin_gps.c +++ b/drivers/usb/serial/garmin_gps.c @@ -104,7 +104,7 @@ struct garmin_packet { int seq; /* the real size of the data array, always > 0 */ int size; - __u8 data[]; + __u8 data[] __counted_by(size); }; /* structure used to keep the current state of the driver */
Use the __counted_by compiler attribute for the data[] flexible array member to improve the results of array bound sanitizers. Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> --- The size is assigned before the first reference to the flexible array (see pkt_add()), which allows for a straightforward annotation without further modifications. --- drivers/usb/serial/garmin_gps.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- base-commit: 0c52056d9f77508cb6d4d68d3fc91c6c08ec71af change-id: 20240619-garmin_gps_counted_by-376545960353 Best regards,