From: Dan Carpenter <dan.carpenter@oracle.com>
To: Souptick Joarder <jrdr.linux@gmail.com>
Cc: mporter@kernel.crashing.org, alex.bou9@gmail.com,
akpm@linux-foundation.org, gustavoars@kernel.org,
jhubbard@nvidia.com, madhuparnabhowmik10@gmail.com,
linux-kernel@vger.kernel.org, Ira Weiny <ira.weiny@intel.com>,
Matthew Wilcox <willy@infradead.org>
Subject: Re: [linux-next PATCH] rapidio: Fix error handling path
Date: Wed, 16 Sep 2020 13:02:32 +0300 [thread overview]
Message-ID: <20200916100232.GF18329@kadam> (raw)
In-Reply-To: <1600227737-20785-1-git-send-email-jrdr.linux@gmail.com>
On Wed, Sep 16, 2020 at 09:12:17AM +0530, Souptick Joarder wrote:
> There is an error when pin_user_pages_fast() returns -ERRNO and
> inside error handling path driver end up calling unpin_user_pages()
> with -ERRNO which is not correct.
>
> This patch will fix the problem.
There are a few ways we could prevent bug in the future.
1) This could have been caught with existing static analysis tools
which warn about when a value is set but not used.
2) I've created a Smatch check which warngs about:
drivers/rapidio/devices/rio_mport_cdev.c:955 rio_dma_transfer() warn: unpinning negative pages 'nr_pages'
I'll test it out tonight and see how well it works. I don't
immediately see any other bugs allthough Smatch doesn't like the code
in siw_umem_release(). It uses "min_t(int" which suggests that
negative pages are okay.
int to_free = min_t(int, PAGES_PER_CHUNK, num_pages);
3) We could add a check in unpin_user_pages().
if (WARN_ON(IS_ERR_VALUE(npages)))
return;
It's not possible to pin more than "ULONG_MAX - 4095" because otherwise
returning error pointers wouldn't work. So this check can't break
anything and it could prevent a crash.
regards,
dan carpenter
next prev parent reply other threads:[~2020-09-16 10:03 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-16 3:42 [linux-next PATCH] rapidio: Fix error handling path Souptick Joarder
2020-09-16 6:37 ` John Hubbard
2020-09-16 10:02 ` Dan Carpenter [this message]
2020-09-16 10:07 ` Dan Carpenter
2020-09-16 15:16 ` Ira Weiny
2020-09-16 15:27 ` Dan Carpenter
2020-09-17 6:57 ` [PATCH] mm/gup: protect unpin_user_pages() against npages==-ERRNO John Hubbard
2020-09-17 7:40 ` Dan Carpenter
2020-09-20 3:03 ` Souptick Joarder
2020-09-20 4:13 ` John Hubbard
2020-09-21 9:34 ` Dan Carpenter
2020-09-17 12:39 ` [linux-next PATCH] rapidio: Fix error handling path Dan Carpenter
2020-09-17 17:34 ` Ira Weiny
2020-09-17 17:47 ` John Hubbard
2020-09-18 2:21 ` Souptick Joarder
2020-09-18 6:33 ` John Hubbard
2020-09-18 2:25 ` Souptick Joarder
2020-09-18 6:15 ` Dan Carpenter
2020-09-16 15:20 ` Ira Weiny
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200916100232.GF18329@kadam \
--to=dan.carpenter@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=alex.bou9@gmail.com \
--cc=gustavoars@kernel.org \
--cc=ira.weiny@intel.com \
--cc=jhubbard@nvidia.com \
--cc=jrdr.linux@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=madhuparnabhowmik10@gmail.com \
--cc=mporter@kernel.crashing.org \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.