Messages in this thread | | | Date | Thu, 3 Aug 2023 14:55:46 +0300 | From | Dan Carpenter <> | Subject | Re: [PATCH v1 net-next 2/4] tc: flower: support for SPI |
| |
On Wed, Aug 02, 2023 at 09:07:35PM +0200, Simon Horman wrote: > + Dan Carpenter > > On Tue, Aug 01, 2023 at 07:10:59AM +0530, Ratheesh Kannoth wrote: > > @@ -1894,6 +1915,12 @@ static int fl_set_key(struct net *net, struct nlattr **tb, > > return ret; > > } > > > > + if (tb[TCA_FLOWER_KEY_SPI]) { > > + ret = fl_set_key_spi(tb, key, mask, extack); > > + if (ret) > > + return ret; > > + } > > + > > Hi Dan, > > I'm seeing a warning from Smatch, which I think is a false positive, > but I feel that I should raise. Perhaps you could take a look at it? > > net/sched/cls_flower.c:1918 fl_set_key() error: buffer overflow 'tb' 106 <= 108 >
You're using the cross function database, right? What happens is that when someone adds a new type of net link attribute, it takes a rebuild for the database to sync up.
I can't think of a good way to fix this. This information is passed as a BUF_SIZE. Each database rebuild passes the BUF_SIZE one call further down the call tree.
$ smdb fl_set_key | grep BUF_SIZE net/sched/cls_flower.c | fl_change | fl_set_key | BUF_SIZE | 1 | tb | 864 net/sched/cls_flower.c | fl_tmplt_create | fl_set_key | BUF_SIZE | 1 | tb | 864
This is a flaw in how Smatch works, and theoretically it affects everything, but in practical terms it affect netlink attribute tables the most. Other places are not modified as often or they pass the size as a parameter. I could modify check_index_overflow.c to silence warnings where it's a netlink attribute table and the offset is less than __TCA_FLOWER_MAX.
regards, dan carpenter
| |