* Stefan Hajnoczi (stefa...@redhat.com) wrote:
> On Tue, Feb 09, 2021 at 07:02:10PM +0000, Dr. David Alan Gilbert (git) wrote:
> > +static void do_removemapping(fuse_req_t req, fuse_ino_t nodeid,
> > + struct fuse_mbuf_iter *iter)
> > +{
> > + struct fuse_removemapping_in *arg;
> > + struct fuse_removemapping_one *one;
> > +
> > + arg = fuse_mbuf_iter_advance(iter, sizeof(*arg));
> > + if (!arg || arg->count <= 0) {
>
> arg->count is unsigned so < is tautologous.
>
> > + fuse_log(FUSE_LOG_ERR, "do_removemapping: invalid arg %p\n", arg);
> > + fuse_reply_err(req, EINVAL);
> > + return;
> > + }
> > +
> > + one = fuse_mbuf_iter_advance(iter, arg->count * sizeof(*one));
>
> arg->count * sizeof(*one) is an integer overflow on 32-bit hosts. I
> think we should be more defensive here since this input comes from the
> guest.
OK, so I've gone with:
if (!arg || !arg->count ||
(uint64_t)arg->count * sizeof(*one) >= SIZE_MAX) {
fuse_log(FUSE_LOG_ERR, "do_removemapping: invalid arg %p\n", arg);
fuse_reply_err(req, EINVAL);
return;
}
to fix both of those (the compiler likes to moan on 64bit about
that comparison being always false in the simpler ways I tried it).
Dave
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
