Dan, how are you running smatch? I've been looking at smatch warnings/errors and don't get this error. Do you have a custom smatch checker? On Wed, Dec 4, 2024 at 2:50 PM Alex Markuze <amarkuze@xxxxxxxxxx> wrote: > > I assume a Coccinelle patch can be written, if one doesn't exist yet. > > On Tue, Dec 3, 2024 at 8:29 PM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > > > > On Tue, Dec 03, 2024 at 11:06:50AM -0600, Alex Elder wrote: > > > On 12/3/24 2:19 AM, Dan Carpenter wrote: > > > > Hello Jeff Layton, > > > > > > > > Commit d48464878708 ("ceph: decode interval_sets for delegated inos") > > > > from Nov 15, 2019 (linux-next), leads to the following Smatch static > > > > checker warning: > > > > > > > > fs/ceph/mds_client.c:644 ceph_parse_deleg_inos() > > > > warn: potential user controlled sizeof overflow 'sets * 2 * 8' '0-u32max * 8' > > > > > > > > fs/ceph/mds_client.c > > > > 637 static int ceph_parse_deleg_inos(void **p, void *end, > > > > 638 struct ceph_mds_session *s) > > > > 639 { > > > > 640 u32 sets; > > > > 641 > > > > 642 ceph_decode_32_safe(p, end, sets, bad); > > > > ^^^^ > > > > set to user data here. > > > > > > > > 643 if (sets) > > > > --> 644 ceph_decode_skip_n(p, end, sets * 2 * sizeof(__le64), bad); > > > > ^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > This is safe on 64bit but on 32bit systems it can integer overflow/wrap. > > > > > > So the point of this is that "sets" is u32, and because that is > > > multiplied by 16 when passed to ceph_decode_skip_n(), the result > > > could exceed 32 bits? I.e., would this address it? > > > > > > if (sets) { > > > size_t scale = 2 * sizeof(__le64); > > > > > > if (sets < SIZE_MAX / scale) > > > ceph_decode_skip_n(p, end, sets * scale, bad); > > > else > > > goto bad; > > > } > > > > > > > Yes, that works. I don't know if there are any static checker warnings which > > will complain that the "sets < SIZE_MAX / scale" is always true on 64 bit. I > > don't think there is? > > > > regards, > > dan carpenter > > > >