Fix misuse of input argument in traverse_visitbp #3348
Conversation
|
fix #2060 I edited it using github online editor, so I didn't even compile test it. |
|
@tuxoko Good idea. As I suspect you're aware, the code is identical in both illumos and FreeBSD. I'm still trying to get a grip on the actual bug here. Specifically, why it's bad that the modified dnp is used by the newly-added IO error handling code but is ok to use in the TRAVERSE_POST callback above. |
|
@dweeezil It's not that it's ok to use in TRAVERSE_POST. It's that no user of TRAVERSE_POST use it. |
In traverse_visitbp(), the input argument dnp is modified in the middle to point to a temporary buffer. Originally this doesn't matter, because no user of TRAVERSE_POST dereferences it. However, in fbeddd6 a piece of code is added dereferencing dnp after the modification, creating a possible bug. We fix this by creating a new local variable cdnp for the DMU_OT_DNODE case, so we don't modify the input argument. Also we introduce different local variables in the DMU_OT_OBJSET case to prevent confusion between the input argument. Signed-off-by: Chunwei Chen <tuxoko@gmail.com>
|
@tuxoko Nice find. I agree with your analysis of the issue. This looks good to me. |
|
LGTM, merged as: ecfb0b5 Fix misuse of input argument in traverse_visitbp |
|
Thanks for the heads up @kpande committed to FreeBSD as ref 282205. |
|
I can say that I have experienced symptoms of this issue as seen below Apr 13 07:30:10 ip-172-26-0-157 kernel: INFO: task spl_system_task:2179 blocked for more than 120 seconds. I have spoke to @dweeezil and @ryao about this. We have applied this pull request and it appears to resolve the issue. |
In traverse_visitbp(), the input argument dnp is modified in the middle to
point to a temporary buffer. Originally this doesn't matter, because no user
of TRAVERSE_POST dereferences it. However, in fbeddd6 a piece of code is added
dereferencing dnp after the modification, creating a possible bug.
We fix this by creating a new local variable cdnp for the DMU_OT_DNODE case,
so we don't modify the input argument. Also we introduce different local
variables in the DMU_OT_OBJSET case to prevent confusion between the input
argument.
Signed-off-by: Chunwei Chen tuxoko@gmail.com