[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 10/24] DAX: virtiofsd: Add setup/remove mappings fuse command
From: |
Vivek Goyal |
Subject: |
Re: [PATCH 10/24] DAX: virtiofsd: Add setup/remove mappings fuse commands |
Date: |
Thu, 11 Feb 2021 15:15:18 -0500 |
On Thu, Feb 11, 2021 at 07:50:37PM +0000, Dr. David Alan Gilbert wrote:
> * Vivek Goyal (vgoyal@redhat.com) wrote:
> > On Thu, Feb 11, 2021 at 04:39:22PM +0000, Dr. David Alan Gilbert wrote:
> > > * Stefan Hajnoczi (stefanha@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;
> >
> > If we did not want to get into unit64_t business, can we alternatively do.
> > if (!arg || !arg->count || arg->count > SIZE_MAX/sizeof(*one)) {
>
> I tried that and the compiler moaned that it was always false; which on
> a 64bit host it is since arg->count is uint32_t.
Hmm.... May be something like.
bool is_arg_count_valid()
{
if (!arg->count)
return false;
#if __WORDSIZE == 64
return true;
#elif
if (argc->count > SIZE_MAX/sizeof(*one))
return false;
#fi
return true;
}
if (!argc || !is_arg_count_valie()) {
}
Vivek
- Re: [PATCH 07/24] DAX: virtio-fs: Add vhost-user slave commands for mapping, (continued)
[PATCH 12/24] DAX: virtiofsd: Wire up passthrough_ll's lo_setupmapping, Dr. David Alan Gilbert (git), 2021/02/09
[PATCH 11/24] DAX: virtiofsd: Add setup/remove mapping handlers to passthrough_ll, Dr. David Alan Gilbert (git), 2021/02/09
[PATCH 13/24] DAX: virtiofsd: Make lo_removemapping() work, Dr. David Alan Gilbert (git), 2021/02/09
[PATCH 14/24] DAX: virtiofsd: route se down to destroy method, Dr. David Alan Gilbert (git), 2021/02/09
[PATCH 15/24] DAX: virtiofsd: Perform an unmap on destroy, Dr. David Alan Gilbert (git), 2021/02/09