Thread (6 messages) 6 messages, 2 authors, 2017-12-05

Re: [PATCH V4] scsi_debugfs: fix crash in scsi_show_rq()

From: Ming Lei <hidden>
Date: 2017-12-05 16:56:20
Also in: linux-scsi

On Wed, Nov 15, 2017 at 08:04:49PM +0800, Ming Lei wrote:
On Wed, Nov 15, 2017 at 07:28:00PM +0900, James Bottomley wrote:
quoted
On Wed, 2017-11-15 at 18:09 +0800, Ming Lei wrote:
quoted
On Tue, Nov 14, 2017 at 10:14:52AM -0800, James Bottomley wrote:
quoted
On Tue, 2017-11-14 at 08:55 +0800, Ming Lei wrote:
quoted
Hi James,

On Mon, Nov 13, 2017 at 10:55:52AM -0800, James Bottomley wrote:
quoted

On Sat, 2017-11-11 at 10:43 +0800, Ming Lei wrote:
quoted

So from CPU1's review, cmd->cmnd is in a remote NUMA node,
__scsi_format_command() is executed much slower than
mempool_free().
So when mempool_free() returns, __scsi_format_command() may
not fetched the buffer in L1 cache yet, then use-after-free
is still triggered.

That is why I say this use-after-free is inevitable no matter
'setting SCpnt->cmnd to NULL before calling mempool_free()'
or not.
The bottom line is that there are several creative ways around
this but the proposed code is currently broken and simply
putting a comment in saying so doesn't make it acceptable.
As I explained above, I didn't see one really workable way. Or
please correct it if I am wrong.
I simply can't believe it's beyond the wit of man to solve a use
after free race. �About 40% of kernel techniques are devoted to
this. �All I really care about is not losing the PI information we
previously had. �I agree with Bart that NULL cmnd is a good
indicator, so it seems reasonable to use it. �If you have another
mechanism, feel free to propose it.
Hi James,

This patch is my proposal, no others thought of yet.

We can fix the use-after-free easily via lock, rcu and ..., but some
cost has to pay. In this case, we can't wait too long in show_rq(),
otherwise we may lose important debug info, so I do not have better
way.

IMO this use-after-free is actually no harm, I don't think we have to
fix it, but it should be better to not let utility warn on this case.
Fine, so lose the snide comment and set the cmnd to NULL *before*
calling free so we narrow the race window.
Hi James,

Given we can't avoid the use-after-free, how about not do that way so
we won't lose the precious debug info too early?
Hi James,

Are you fine with V4?

As I explained, the use-after-free can't be avoided, we have to make
scsi_show_rq() to survive that, so we don't need to touch code in free path.
Also we won't lose debug info too early in this way, not like 'set the cmnd
to NULL *before* calling free'.

Thanks,
Ming
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help