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
