On Wed, Nov 27, 2019 at 02:36:01AM +0000, Yu, Jin wrote:
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richard...@intel.com>
> > Sent: Tuesday, November 26, 2019 6:26 PM
> > To: Yu, Jin <jin...@intel.com>
> > Cc: Maxime Coquelin <maxime.coque...@redhat.com>; Bie, Tiwei
> > <tiwei....@intel.com>; Wang, Zhihong <zhihong.w...@intel.com>;
> > dev@dpdk.org; sta...@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] examples/vhost_blk: fix the TOCTOU
> >
> > On Tue, Nov 26, 2019 at 11:32:14PM +0800, Jin Yu wrote:
> > > Fix the time of check time of use warning in example code
> > >
> > > Coverity issue: 350589 158663
> > > Fixes: c19beb3f38cd ("examples/vhost_blk: introduce vhost storage
> > > sample")
> > > Cc: sta...@dpdk.org
> > >
> > > Signed-off-by: Jin Yu <jin...@intel.com>
> > > ---
> > > examples/vhost_blk/vhost_blk.c | 9 ++-------
> > > 1 file changed, 2 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/examples/vhost_blk/vhost_blk.c
> > > b/examples/vhost_blk/vhost_blk.c index 3182a488b..bcb4ebb0b 100644
> > > --- a/examples/vhost_blk/vhost_blk.c
> > > +++ b/examples/vhost_blk/vhost_blk.c
> > > @@ -993,11 +993,7 @@ vhost_blk_ctrlr_construct(const char *ctrlr_name)
> > > }
> > > snprintf(dev_pathname, sizeof(dev_pathname), "%s/%s", path,
> > > ctrlr_name);
> > >
> > > - if (access(dev_pathname, F_OK) != -1) {
> > > - if (unlink(dev_pathname) != 0)
> > > - rte_exit(EXIT_FAILURE, "Cannot remove %s.\n",
> > > - dev_pathname);
> > > - }
> > > + unlink(dev_pathname);
> > >
> >
> > The original code did an exit if the delete failed, do you intend there to
> > be a
> > behaviour change here? You can probably get the same behaviour if you
> > check the errno on an unlink failure, e.g. ENOENT means file doesn't exist.
> >
> > If not having the app exit on unlink failure is reasonable behaviour then
> > ignore this comment.
>
> I considered it. I think it's ok to ignore the errno of unlink failure. This
> code just want
> to remove the file. There are two situations. The first one is that file
> doesn't exist the unlink
> fails and it's ok to ignore. The second one is that unlink fails to remove
> file but the next bind()
> would fail too so I think it's ok to ignore too.
> >
Ok, thanks for clarifying.
