* [Ocfs2-devel] [PATCH] ocfs2: fix a NULL pointer dereference when call ocfs2_update_inode_fsync_trans()
@ 2020-01-08 9:23 wangyan
2020-01-08 11:31 ` Joseph Qi
2020-01-09 1:57 ` Changwei Ge
0 siblings, 2 replies; 7+ messages in thread
From: wangyan @ 2020-01-08 9:23 UTC (permalink / raw)
To: ocfs2-devel
I found a NULL pointer dereference in ocfs2_update_inode_fsync_trans(),
handle->h_transaction may be NULL in this situation:
ocfs2_file_write_iter
->__generic_file_write_iter
->generic_perform_write
->ocfs2_write_begin
->ocfs2_write_begin_nolock
->ocfs2_write_cluster_by_desc
->ocfs2_write_cluster
->ocfs2_mark_extent_written
->ocfs2_change_extent_flag
->ocfs2_split_extent
->ocfs2_try_to_merge_extent
->ocfs2_extend_rotate_transaction
->ocfs2_extend_trans
->jbd2_journal_restart
->jbd2__journal_restart
// handle->h_transaction is NULL here
->handle->h_transaction = NULL;
->start_this_handle
/* journal aborted due to storage
network disconnection, return error */
->return -EROFS;
/* line 3806 in ocfs2_try_to_merge_extent (),
it will ignore ret error. */
->ret = 0;
->...
->ocfs2_write_end
->ocfs2_write_end_nolock
->ocfs2_update_inode_fsync_trans
// NULL pointer dereference
->oi->i_sync_tid = handle->h_transaction->t_tid;
The information of NULL pointer dereference as follows:
JBD2: Detected IO errors while flushing file data on dm-11-45
Aborting journal on device dm-11-45.
JBD2: Error -5 detected when updating journal superblock for dm-11-45.
(dd,22081,3):ocfs2_extend_trans:474 ERROR: status = -30
(dd,22081,3):ocfs2_try_to_merge_extent:3877 ERROR: status = -30
Unable to handle kernel NULL pointer dereference at
virtual address 0000000000000008
Mem abort info:
ESR = 0x96000004
Exception class = DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
Data abort info:
ISV = 0, ISS = 0x00000004
CM = 0, WnR = 0
user pgtable: 4k pages, 48-bit VAs, pgdp = 00000000e74e1338
[0000000000000008] pgd=0000000000000000
Internal error: Oops: 96000004 [#1] SMP
Process dd (pid: 22081, stack limit = 0x00000000584f35a9)
CPU: 3 PID: 22081 Comm: dd Kdump: loaded
Hardware name: Huawei TaiShan 2280 V2/BC82AMDD, BIOS 0.98 08/25/2019
pstate: 60400009 (nZCv daif +PAN -UAO)
pc : ocfs2_write_end_nolock+0x2b8/0x550 [ocfs2]
lr : ocfs2_write_end_nolock+0x2a0/0x550 [ocfs2]
sp : ffff0000459fba70
x29: ffff0000459fba70 x28: 0000000000000000
x27: ffff807ccf7f1000 x26: 0000000000000001
x25: ffff807bdff57970 x24: ffff807caf1d4000
x23: ffff807cc79e9000 x22: 0000000000001000
x21: 000000006c6cd000 x20: ffff0000091d9000
x19: ffff807ccb239db0 x18: ffffffffffffffff
x17: 000000000000000e x16: 0000000000000007
x15: ffff807c5e15bd78 x14: 0000000000000000
x13: 0000000000000000 x12: 0000000000000000
x11: 0000000000000000 x10: 0000000000000001
x9 : 0000000000000228 x8 : 000000000000000c
x7 : 0000000000000fff x6 : ffff807a308ed6b0
x5 : ffff7e01f10967c0 x4 : 0000000000000018
x3 : d0bc661572445600 x2 : 0000000000000000
x1 : 000000001b2e0200 x0 : 0000000000000000
Call trace:
ocfs2_write_end_nolock+0x2b8/0x550 [ocfs2]
ocfs2_write_end+0x4c/0x80 [ocfs2]
generic_perform_write+0x108/0x1a8
__generic_file_write_iter+0x158/0x1c8
ocfs2_file_write_iter+0x668/0x950 [ocfs2]
__vfs_write+0x11c/0x190
vfs_write+0xac/0x1c0
ksys_write+0x6c/0xd8
__arm64_sys_write+0x24/0x30
el0_svc_common+0x78/0x130
el0_svc_handler+0x38/0x78
el0_svc+0x8/0xc
To prevent NULL pointer dereference in this situation, we use
is_handle_aborted() before using handle->h_transaction->t_tid.
Signed-off-by: Yan Wang <wangyan122@huawei.com>
Reviewed-by: Jun Piao <piaojun@huawei.com>
---
fs/ocfs2/journal.h | 8 +++++---
fs/ocfs2/namei.c | 3 +--
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/fs/ocfs2/journal.h b/fs/ocfs2/journal.h
index 3103ba7f97a2..bfe611ed1b1d 100644
--- a/fs/ocfs2/journal.h
+++ b/fs/ocfs2/journal.h
@@ -597,9 +597,11 @@ static inline void
ocfs2_update_inode_fsync_trans(handle_t *handle,
{
struct ocfs2_inode_info *oi = OCFS2_I(inode);
- oi->i_sync_tid = handle->h_transaction->t_tid;
- if (datasync)
- oi->i_datasync_tid = handle->h_transaction->t_tid;
+ if (!is_handle_aborted(handle)) {
+ oi->i_sync_tid = handle->h_transaction->t_tid;
+ if (datasync)
+ oi->i_datasync_tid = handle->h_transaction->t_tid;
+ }
}
#endif /* OCFS2_JOURNAL_H */
diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index 8ea51cf27b97..da65251ef815 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -586,8 +586,7 @@ static int __ocfs2_mknod_locked(struct inode *dir,
mlog_errno(status);
}
- oi->i_sync_tid = handle->h_transaction->t_tid;
- oi->i_datasync_tid = handle->h_transaction->t_tid;
+ ocfs2_update_inode_fsync_trans(handle, inode, 1);
leave:
if (status < 0) {
--
2.19.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: fix a NULL pointer dereference when call ocfs2_update_inode_fsync_trans()
2020-01-08 9:23 [Ocfs2-devel] [PATCH] ocfs2: fix a NULL pointer dereference when call ocfs2_update_inode_fsync_trans() wangyan
@ 2020-01-08 11:31 ` Joseph Qi
2020-01-08 12:14 ` wangyan
2020-01-09 1:57 ` Changwei Ge
1 sibling, 1 reply; 7+ messages in thread
From: Joseph Qi @ 2020-01-08 11:31 UTC (permalink / raw)
To: ocfs2-devel
On 20/1/8 17:23, wangyan wrote:
> I found a NULL pointer dereference in ocfs2_update_inode_fsync_trans(),
> handle->h_transaction may be NULL in this situation:
> ocfs2_file_write_iter
> ? ->__generic_file_write_iter
> ????? ->generic_perform_write
> ??????? ->ocfs2_write_begin
> ????????? ->ocfs2_write_begin_nolock
> ??????????? ->ocfs2_write_cluster_by_desc
> ????????????? ->ocfs2_write_cluster
> ??????????????? ->ocfs2_mark_extent_written
> ????????????????? ->ocfs2_change_extent_flag
> ??????????????????? ->ocfs2_split_extent
> ????????????????????? ->ocfs2_try_to_merge_extent
> ??????????????????????? ->ocfs2_extend_rotate_transaction
> ????????????????????????? ->ocfs2_extend_trans
> ??????????????????????????? ->jbd2_journal_restart
> ????????????????????????????? ->jbd2__journal_restart
> ??????????????????????????????? // handle->h_transaction is NULL here
> ??????????????????????????????? ->handle->h_transaction = NULL;
> ??????????????????????????????? ->start_this_handle
> ????????????????????????????????? /* journal aborted due to storage
> ???????????????????????????????????? network disconnection, return error */
> ????????????????????????????????? ->return -EROFS;
> ???????????????????????? /* line 3806 in ocfs2_try_to_merge_extent (),
> ??????????????????????????? it will ignore ret error. */
> ??????????????????????? ->ret = 0;
> ??????? ->...
> ??????? ->ocfs2_write_end
> ????????? ->ocfs2_write_end_nolock
> ??????????? ->ocfs2_update_inode_fsync_trans
> ????????????? // NULL pointer dereference
> ????????????? ->oi->i_sync_tid = handle->h_transaction->t_tid;
>
> The information of NULL pointer dereference as follows:
> ??? JBD2: Detected IO errors while flushing file data on dm-11-45
> ??? Aborting journal on device dm-11-45.
> ??? JBD2: Error -5 detected when updating journal superblock for dm-11-45.
> ??? (dd,22081,3):ocfs2_extend_trans:474 ERROR: status = -30
> ??? (dd,22081,3):ocfs2_try_to_merge_extent:3877 ERROR: status = -30
> ??? Unable to handle kernel NULL pointer dereference at
> ??? virtual address 0000000000000008
> ??? Mem abort info:
> ????? ESR = 0x96000004
> ????? Exception class = DABT (current EL), IL = 32 bits
> ????? SET = 0, FnV = 0
> ????? EA = 0, S1PTW = 0
> ??? Data abort info:
> ????? ISV = 0, ISS = 0x00000004
> ????? CM = 0, WnR = 0
> ??? user pgtable: 4k pages, 48-bit VAs, pgdp = 00000000e74e1338
> ??? [0000000000000008] pgd=0000000000000000
> ??? Internal error: Oops: 96000004 [#1] SMP
> ??? Process dd (pid: 22081, stack limit = 0x00000000584f35a9)
> ??? CPU: 3 PID: 22081 Comm: dd Kdump: loaded
> ??? Hardware name: Huawei TaiShan 2280 V2/BC82AMDD, BIOS 0.98 08/25/2019
> ??? pstate: 60400009 (nZCv daif +PAN -UAO)
> ??? pc : ocfs2_write_end_nolock+0x2b8/0x550 [ocfs2]
> ??? lr : ocfs2_write_end_nolock+0x2a0/0x550 [ocfs2]
> ??? sp : ffff0000459fba70
> ??? x29: ffff0000459fba70 x28: 0000000000000000
> ??? x27: ffff807ccf7f1000 x26: 0000000000000001
> ??? x25: ffff807bdff57970 x24: ffff807caf1d4000
> ??? x23: ffff807cc79e9000 x22: 0000000000001000
> ??? x21: 000000006c6cd000 x20: ffff0000091d9000
> ??? x19: ffff807ccb239db0 x18: ffffffffffffffff
> ??? x17: 000000000000000e x16: 0000000000000007
> ??? x15: ffff807c5e15bd78 x14: 0000000000000000
> ??? x13: 0000000000000000 x12: 0000000000000000
> ??? x11: 0000000000000000 x10: 0000000000000001
> ??? x9 : 0000000000000228 x8 : 000000000000000c
> ??? x7 : 0000000000000fff x6 : ffff807a308ed6b0
> ??? x5 : ffff7e01f10967c0 x4 : 0000000000000018
> ??? x3 : d0bc661572445600 x2 : 0000000000000000
> ??? x1 : 000000001b2e0200 x0 : 0000000000000000
> ??? Call trace:
> ???? ocfs2_write_end_nolock+0x2b8/0x550 [ocfs2]
> ???? ocfs2_write_end+0x4c/0x80 [ocfs2]
> ???? generic_perform_write+0x108/0x1a8
> ???? __generic_file_write_iter+0x158/0x1c8
> ???? ocfs2_file_write_iter+0x668/0x950 [ocfs2]
> ???? __vfs_write+0x11c/0x190
> ???? vfs_write+0xac/0x1c0
> ???? ksys_write+0x6c/0xd8
> ???? __arm64_sys_write+0x24/0x30
> ???? el0_svc_common+0x78/0x130
> ???? el0_svc_handler+0x38/0x78
> ???? el0_svc+0x8/0xc
>
> To prevent NULL pointer dereference in this situation, we use
> is_handle_aborted() before using handle->h_transaction->t_tid.
>
> Signed-off-by: Yan Wang <wangyan122@huawei.com>
> Reviewed-by: Jun Piao <piaojun@huawei.com>
> ---
> ?fs/ocfs2/journal.h | 8 +++++---
> ?fs/ocfs2/namei.c?? | 3 +--
> ?2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ocfs2/journal.h b/fs/ocfs2/journal.h
> index 3103ba7f97a2..bfe611ed1b1d 100644
> --- a/fs/ocfs2/journal.h
> +++ b/fs/ocfs2/journal.h
> @@ -597,9 +597,11 @@ static inline void ocfs2_update_inode_fsync_trans(handle_t *handle,
> ?{
> ???? struct ocfs2_inode_info *oi = OCFS2_I(inode);
>
> -??? oi->i_sync_tid = handle->h_transaction->t_tid;
> -??? if (datasync)
> -??????? oi->i_datasync_tid = handle->h_transaction->t_tid;
> +??? if (!is_handle_aborted(handle)) {
> +??????? oi->i_sync_tid = handle->h_transaction->t_tid;
> +??????? if (datasync)
> +??????????? oi->i_datasync_tid = handle->h_transaction->t_tid;
Use tab instead of space, please.
> +??? }
> ?}
>
> ?#endif /* OCFS2_JOURNAL_H */
> diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
> index 8ea51cf27b97..da65251ef815 100644
> --- a/fs/ocfs2/namei.c
> +++ b/fs/ocfs2/namei.c
> @@ -586,8 +586,7 @@ static int __ocfs2_mknod_locked(struct inode *dir,
> ???????????? mlog_errno(status);
> ???? }
>
> -??? oi->i_sync_tid = handle->h_transaction->t_tid;
> -??? oi->i_datasync_tid = handle->h_transaction->t_tid;
> +??? ocfs2_update_inode_fsync_trans(handle, inode, 1);
>
I don't see any reason why we have to check handle here.
Thanks,
Joseph
> ?leave:
> ???? if (status < 0) {
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: fix a NULL pointer dereference when call ocfs2_update_inode_fsync_trans()
2020-01-08 11:31 ` Joseph Qi
@ 2020-01-08 12:14 ` wangyan
2020-01-09 1:25 ` Joseph Qi
0 siblings, 1 reply; 7+ messages in thread
From: wangyan @ 2020-01-08 12:14 UTC (permalink / raw)
To: ocfs2-devel
On 2020/1/8 19:31, Joseph Qi wrote:
>
>
> On 20/1/8 17:23, wangyan wrote:
>> I found a NULL pointer dereference in ocfs2_update_inode_fsync_trans(),
>> handle->h_transaction may be NULL in this situation:
>> ocfs2_file_write_iter
>> ->__generic_file_write_iter
>> ->generic_perform_write
>> ->ocfs2_write_begin
>> ->ocfs2_write_begin_nolock
>> ->ocfs2_write_cluster_by_desc
>> ->ocfs2_write_cluster
>> ->ocfs2_mark_extent_written
>> ->ocfs2_change_extent_flag
>> ->ocfs2_split_extent
>> ->ocfs2_try_to_merge_extent
>> ->ocfs2_extend_rotate_transaction
>> ->ocfs2_extend_trans
>> ->jbd2_journal_restart
>> ->jbd2__journal_restart
>> // handle->h_transaction is NULL here
>> ->handle->h_transaction = NULL;
>> ->start_this_handle
>> /* journal aborted due to storage
>> network disconnection, return error */
>> ->return -EROFS;
>> /* line 3806 in ocfs2_try_to_merge_extent (),
>> it will ignore ret error. */
>> ->ret = 0;
>> ->...
>> ->ocfs2_write_end
>> ->ocfs2_write_end_nolock
>> ->ocfs2_update_inode_fsync_trans
>> // NULL pointer dereference
>> ->oi->i_sync_tid = handle->h_transaction->t_tid;
>>
>> The information of NULL pointer dereference as follows:
>> JBD2: Detected IO errors while flushing file data on dm-11-45
>> Aborting journal on device dm-11-45.
>> JBD2: Error -5 detected when updating journal superblock for dm-11-45.
>> (dd,22081,3):ocfs2_extend_trans:474 ERROR: status = -30
>> (dd,22081,3):ocfs2_try_to_merge_extent:3877 ERROR: status = -30
>> Unable to handle kernel NULL pointer dereference at
>> virtual address 0000000000000008
>> Mem abort info:
>> ESR = 0x96000004
>> Exception class = DABT (current EL), IL = 32 bits
>> SET = 0, FnV = 0
>> EA = 0, S1PTW = 0
>> Data abort info:
>> ISV = 0, ISS = 0x00000004
>> CM = 0, WnR = 0
>> user pgtable: 4k pages, 48-bit VAs, pgdp = 00000000e74e1338
>> [0000000000000008] pgd=0000000000000000
>> Internal error: Oops: 96000004 [#1] SMP
>> Process dd (pid: 22081, stack limit = 0x00000000584f35a9)
>> CPU: 3 PID: 22081 Comm: dd Kdump: loaded
>> Hardware name: Huawei TaiShan 2280 V2/BC82AMDD, BIOS 0.98 08/25/2019
>> pstate: 60400009 (nZCv daif +PAN -UAO)
>> pc : ocfs2_write_end_nolock+0x2b8/0x550 [ocfs2]
>> lr : ocfs2_write_end_nolock+0x2a0/0x550 [ocfs2]
>> sp : ffff0000459fba70
>> x29: ffff0000459fba70 x28: 0000000000000000
>> x27: ffff807ccf7f1000 x26: 0000000000000001
>> x25: ffff807bdff57970 x24: ffff807caf1d4000
>> x23: ffff807cc79e9000 x22: 0000000000001000
>> x21: 000000006c6cd000 x20: ffff0000091d9000
>> x19: ffff807ccb239db0 x18: ffffffffffffffff
>> x17: 000000000000000e x16: 0000000000000007
>> x15: ffff807c5e15bd78 x14: 0000000000000000
>> x13: 0000000000000000 x12: 0000000000000000
>> x11: 0000000000000000 x10: 0000000000000001
>> x9 : 0000000000000228 x8 : 000000000000000c
>> x7 : 0000000000000fff x6 : ffff807a308ed6b0
>> x5 : ffff7e01f10967c0 x4 : 0000000000000018
>> x3 : d0bc661572445600 x2 : 0000000000000000
>> x1 : 000000001b2e0200 x0 : 0000000000000000
>> Call trace:
>> ocfs2_write_end_nolock+0x2b8/0x550 [ocfs2]
>> ocfs2_write_end+0x4c/0x80 [ocfs2]
>> generic_perform_write+0x108/0x1a8
>> __generic_file_write_iter+0x158/0x1c8
>> ocfs2_file_write_iter+0x668/0x950 [ocfs2]
>> __vfs_write+0x11c/0x190
>> vfs_write+0xac/0x1c0
>> ksys_write+0x6c/0xd8
>> __arm64_sys_write+0x24/0x30
>> el0_svc_common+0x78/0x130
>> el0_svc_handler+0x38/0x78
>> el0_svc+0x8/0xc
>>
>> To prevent NULL pointer dereference in this situation, we use
>> is_handle_aborted() before using handle->h_transaction->t_tid.
>>
>> Signed-off-by: Yan Wang <wangyan122@huawei.com>
>> Reviewed-by: Jun Piao <piaojun@huawei.com>
>> ---
>> fs/ocfs2/journal.h | 8 +++++---
>> fs/ocfs2/namei.c | 3 +--
>> 2 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/ocfs2/journal.h b/fs/ocfs2/journal.h
>> index 3103ba7f97a2..bfe611ed1b1d 100644
>> --- a/fs/ocfs2/journal.h
>> +++ b/fs/ocfs2/journal.h
>> @@ -597,9 +597,11 @@ static inline void ocfs2_update_inode_fsync_trans(handle_t *handle,
>> {
>> struct ocfs2_inode_info *oi = OCFS2_I(inode);
>>
>> - oi->i_sync_tid = handle->h_transaction->t_tid;
>> - if (datasync)
>> - oi->i_datasync_tid = handle->h_transaction->t_tid;
>> + if (!is_handle_aborted(handle)) {
>> + oi->i_sync_tid = handle->h_transaction->t_tid;
>> + if (datasync)
>> + oi->i_datasync_tid = handle->h_transaction->t_tid;
>
> Use tab instead of space, please.
Thanks, I will resend it later.
>
>> + }
>> }
>>
>> #endif /* OCFS2_JOURNAL_H */
>> diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
>> index 8ea51cf27b97..da65251ef815 100644
>> --- a/fs/ocfs2/namei.c
>> +++ b/fs/ocfs2/namei.c
>> @@ -586,8 +586,7 @@ static int __ocfs2_mknod_locked(struct inode *dir,
>> mlog_errno(status);
>> }
>>
>> - oi->i_sync_tid = handle->h_transaction->t_tid;
>> - oi->i_datasync_tid = handle->h_transaction->t_tid;
>> + ocfs2_update_inode_fsync_trans(handle, inode, 1);
>>
> I don't see any reason why we have to check handle here.
>
> Thanks,
> Joseph
Yes, it doesn't need to check handle here.
handle->h_transaction can not be NULL here. But?other
functions use ocfs2_update_inode_fsync_trans() to access
t_tid in handle->h_transaction.
I modify it for the uniform format.
Thanks,
Yan Wang
>
>> leave:
>> if (status < 0) {
>
> .
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: fix a NULL pointer dereference when call ocfs2_update_inode_fsync_trans()
2020-01-08 12:14 ` wangyan
@ 2020-01-09 1:25 ` Joseph Qi
2020-01-09 1:37 ` wangyan
0 siblings, 1 reply; 7+ messages in thread
From: Joseph Qi @ 2020-01-09 1:25 UTC (permalink / raw)
To: ocfs2-devel
On 20/1/8 20:14, wangyan wrote:
> On 2020/1/8 19:31, Joseph Qi wrote:
>>
>>
>> On 20/1/8 17:23, wangyan wrote:
>>> I found a NULL pointer dereference in ocfs2_update_inode_fsync_trans(),
>>> handle->h_transaction may be NULL in this situation:
>>> ocfs2_file_write_iter
>>> ? ->__generic_file_write_iter
>>> ????? ->generic_perform_write
>>> ??????? ->ocfs2_write_begin
>>> ????????? ->ocfs2_write_begin_nolock
>>> ??????????? ->ocfs2_write_cluster_by_desc
>>> ????????????? ->ocfs2_write_cluster
>>> ??????????????? ->ocfs2_mark_extent_written
>>> ????????????????? ->ocfs2_change_extent_flag
>>> ??????????????????? ->ocfs2_split_extent
>>> ????????????????????? ->ocfs2_try_to_merge_extent
>>> ??????????????????????? ->ocfs2_extend_rotate_transaction
>>> ????????????????????????? ->ocfs2_extend_trans
>>> ??????????????????????????? ->jbd2_journal_restart
>>> ????????????????????????????? ->jbd2__journal_restart
>>> ??????????????????????????????? // handle->h_transaction is NULL here
>>> ??????????????????????????????? ->handle->h_transaction = NULL;
>>> ??????????????????????????????? ->start_this_handle
>>> ????????????????????????????????? /* journal aborted due to storage
>>> ???????????????????????????????????? network disconnection, return error */
>>> ????????????????????????????????? ->return -EROFS;
>>> ???????????????????????? /* line 3806 in ocfs2_try_to_merge_extent (),
>>> ??????????????????????????? it will ignore ret error. */
>>> ??????????????????????? ->ret = 0;
>>> ??????? ->...
>>> ??????? ->ocfs2_write_end
>>> ????????? ->ocfs2_write_end_nolock
>>> ??????????? ->ocfs2_update_inode_fsync_trans
>>> ????????????? // NULL pointer dereference
>>> ????????????? ->oi->i_sync_tid = handle->h_transaction->t_tid;
>>>
>>> The information of NULL pointer dereference as follows:
>>> ??? JBD2: Detected IO errors while flushing file data on dm-11-45
>>> ??? Aborting journal on device dm-11-45.
>>> ??? JBD2: Error -5 detected when updating journal superblock for dm-11-45.
>>> ??? (dd,22081,3):ocfs2_extend_trans:474 ERROR: status = -30
>>> ??? (dd,22081,3):ocfs2_try_to_merge_extent:3877 ERROR: status = -30
>>> ??? Unable to handle kernel NULL pointer dereference at
>>> ??? virtual address 0000000000000008
>>> ??? Mem abort info:
>>> ????? ESR = 0x96000004
>>> ????? Exception class = DABT (current EL), IL = 32 bits
>>> ????? SET = 0, FnV = 0
>>> ????? EA = 0, S1PTW = 0
>>> ??? Data abort info:
>>> ????? ISV = 0, ISS = 0x00000004
>>> ????? CM = 0, WnR = 0
>>> ??? user pgtable: 4k pages, 48-bit VAs, pgdp = 00000000e74e1338
>>> ??? [0000000000000008] pgd=0000000000000000
>>> ??? Internal error: Oops: 96000004 [#1] SMP
>>> ??? Process dd (pid: 22081, stack limit = 0x00000000584f35a9)
>>> ??? CPU: 3 PID: 22081 Comm: dd Kdump: loaded
>>> ??? Hardware name: Huawei TaiShan 2280 V2/BC82AMDD, BIOS 0.98 08/25/2019
>>> ??? pstate: 60400009 (nZCv daif +PAN -UAO)
>>> ??? pc : ocfs2_write_end_nolock+0x2b8/0x550 [ocfs2]
>>> ??? lr : ocfs2_write_end_nolock+0x2a0/0x550 [ocfs2]
>>> ??? sp : ffff0000459fba70
>>> ??? x29: ffff0000459fba70 x28: 0000000000000000
>>> ??? x27: ffff807ccf7f1000 x26: 0000000000000001
>>> ??? x25: ffff807bdff57970 x24: ffff807caf1d4000
>>> ??? x23: ffff807cc79e9000 x22: 0000000000001000
>>> ??? x21: 000000006c6cd000 x20: ffff0000091d9000
>>> ??? x19: ffff807ccb239db0 x18: ffffffffffffffff
>>> ??? x17: 000000000000000e x16: 0000000000000007
>>> ??? x15: ffff807c5e15bd78 x14: 0000000000000000
>>> ??? x13: 0000000000000000 x12: 0000000000000000
>>> ??? x11: 0000000000000000 x10: 0000000000000001
>>> ??? x9 : 0000000000000228 x8 : 000000000000000c
>>> ??? x7 : 0000000000000fff x6 : ffff807a308ed6b0
>>> ??? x5 : ffff7e01f10967c0 x4 : 0000000000000018
>>> ??? x3 : d0bc661572445600 x2 : 0000000000000000
>>> ??? x1 : 000000001b2e0200 x0 : 0000000000000000
>>> ??? Call trace:
>>> ???? ocfs2_write_end_nolock+0x2b8/0x550 [ocfs2]
>>> ???? ocfs2_write_end+0x4c/0x80 [ocfs2]
>>> ???? generic_perform_write+0x108/0x1a8
>>> ???? __generic_file_write_iter+0x158/0x1c8
>>> ???? ocfs2_file_write_iter+0x668/0x950 [ocfs2]
>>> ???? __vfs_write+0x11c/0x190
>>> ???? vfs_write+0xac/0x1c0
>>> ???? ksys_write+0x6c/0xd8
>>> ???? __arm64_sys_write+0x24/0x30
>>> ???? el0_svc_common+0x78/0x130
>>> ???? el0_svc_handler+0x38/0x78
>>> ???? el0_svc+0x8/0xc
>>>
>>> To prevent NULL pointer dereference in this situation, we use
>>> is_handle_aborted() before using handle->h_transaction->t_tid.
>>>
>>> Signed-off-by: Yan Wang <wangyan122@huawei.com>
>>> Reviewed-by: Jun Piao <piaojun@huawei.com>
>>> ---
>>> ?fs/ocfs2/journal.h | 8 +++++---
>>> ?fs/ocfs2/namei.c?? | 3 +--
>>> ?2 files changed, 6 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fs/ocfs2/journal.h b/fs/ocfs2/journal.h
>>> index 3103ba7f97a2..bfe611ed1b1d 100644
>>> --- a/fs/ocfs2/journal.h
>>> +++ b/fs/ocfs2/journal.h
>>> @@ -597,9 +597,11 @@ static inline void ocfs2_update_inode_fsync_trans(handle_t *handle,
>>> ?{
>>> ???? struct ocfs2_inode_info *oi = OCFS2_I(inode);
>>>
>>> -??? oi->i_sync_tid = handle->h_transaction->t_tid;
>>> -??? if (datasync)
>>> -??????? oi->i_datasync_tid = handle->h_transaction->t_tid;
>>> +??? if (!is_handle_aborted(handle)) {
>>> +??????? oi->i_sync_tid = handle->h_transaction->t_tid;
>>> +??????? if (datasync)
>>> +??????????? oi->i_datasync_tid = handle->h_transaction->t_tid;
>>
>> Use tab instead of space, please.
> Thanks, I will resend it later.
>>
>>> +??? }
>>> ?}
>>>
>>> ?#endif /* OCFS2_JOURNAL_H */
>>> diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
>>> index 8ea51cf27b97..da65251ef815 100644
>>> --- a/fs/ocfs2/namei.c
>>> +++ b/fs/ocfs2/namei.c
>>> @@ -586,8 +586,7 @@ static int __ocfs2_mknod_locked(struct inode *dir,
>>> ???????????? mlog_errno(status);
>>> ???? }
>>>
>>> -??? oi->i_sync_tid = handle->h_transaction->t_tid;
>>> -??? oi->i_datasync_tid = handle->h_transaction->t_tid;
>>> +??? ocfs2_update_inode_fsync_trans(handle, inode, 1);
>>>
>> I don't see any reason why we have to check handle here.
>>
>> Thanks,
>> Joseph
> Yes, it doesn't need to check handle here.
> handle->h_transaction can not be NULL here. But?other
> functions use ocfs2_update_inode_fsync_trans() to access
> t_tid in handle->h_transaction.
> I modify it for the uniform format.
>
I suggest we split the above cleanup into another patch. Since it
doesn't have relations with the subject.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: fix a NULL pointer dereference when call ocfs2_update_inode_fsync_trans()
2020-01-09 1:25 ` Joseph Qi
@ 2020-01-09 1:37 ` wangyan
0 siblings, 0 replies; 7+ messages in thread
From: wangyan @ 2020-01-09 1:37 UTC (permalink / raw)
To: ocfs2-devel
On 2020/1/9 9:25, Joseph Qi wrote:
>
>
> On 20/1/8 20:14, wangyan wrote:
>> On 2020/1/8 19:31, Joseph Qi wrote:
>>>
>>>
>>> On 20/1/8 17:23, wangyan wrote:
>>>> I found a NULL pointer dereference in ocfs2_update_inode_fsync_trans(),
>>>> handle->h_transaction may be NULL in this situation:
>>>> ocfs2_file_write_iter
>>>> ->__generic_file_write_iter
>>>> ->generic_perform_write
>>>> ->ocfs2_write_begin
>>>> ->ocfs2_write_begin_nolock
>>>> ->ocfs2_write_cluster_by_desc
>>>> ->ocfs2_write_cluster
>>>> ->ocfs2_mark_extent_written
>>>> ->ocfs2_change_extent_flag
>>>> ->ocfs2_split_extent
>>>> ->ocfs2_try_to_merge_extent
>>>> ->ocfs2_extend_rotate_transaction
>>>> ->ocfs2_extend_trans
>>>> ->jbd2_journal_restart
>>>> ->jbd2__journal_restart
>>>> // handle->h_transaction is NULL here
>>>> ->handle->h_transaction = NULL;
>>>> ->start_this_handle
>>>> /* journal aborted due to storage
>>>> network disconnection, return error */
>>>> ->return -EROFS;
>>>> /* line 3806 in ocfs2_try_to_merge_extent (),
>>>> it will ignore ret error. */
>>>> ->ret = 0;
>>>> ->...
>>>> ->ocfs2_write_end
>>>> ->ocfs2_write_end_nolock
>>>> ->ocfs2_update_inode_fsync_trans
>>>> // NULL pointer dereference
>>>> ->oi->i_sync_tid = handle->h_transaction->t_tid;
>>>>
>>>> The information of NULL pointer dereference as follows:
>>>> JBD2: Detected IO errors while flushing file data on dm-11-45
>>>> Aborting journal on device dm-11-45.
>>>> JBD2: Error -5 detected when updating journal superblock for dm-11-45.
>>>> (dd,22081,3):ocfs2_extend_trans:474 ERROR: status = -30
>>>> (dd,22081,3):ocfs2_try_to_merge_extent:3877 ERROR: status = -30
>>>> Unable to handle kernel NULL pointer dereference at
>>>> virtual address 0000000000000008
>>>> Mem abort info:
>>>> ESR = 0x96000004
>>>> Exception class = DABT (current EL), IL = 32 bits
>>>> SET = 0, FnV = 0
>>>> EA = 0, S1PTW = 0
>>>> Data abort info:
>>>> ISV = 0, ISS = 0x00000004
>>>> CM = 0, WnR = 0
>>>> user pgtable: 4k pages, 48-bit VAs, pgdp = 00000000e74e1338
>>>> [0000000000000008] pgd=0000000000000000
>>>> Internal error: Oops: 96000004 [#1] SMP
>>>> Process dd (pid: 22081, stack limit = 0x00000000584f35a9)
>>>> CPU: 3 PID: 22081 Comm: dd Kdump: loaded
>>>> Hardware name: Huawei TaiShan 2280 V2/BC82AMDD, BIOS 0.98 08/25/2019
>>>> pstate: 60400009 (nZCv daif +PAN -UAO)
>>>> pc : ocfs2_write_end_nolock+0x2b8/0x550 [ocfs2]
>>>> lr : ocfs2_write_end_nolock+0x2a0/0x550 [ocfs2]
>>>> sp : ffff0000459fba70
>>>> x29: ffff0000459fba70 x28: 0000000000000000
>>>> x27: ffff807ccf7f1000 x26: 0000000000000001
>>>> x25: ffff807bdff57970 x24: ffff807caf1d4000
>>>> x23: ffff807cc79e9000 x22: 0000000000001000
>>>> x21: 000000006c6cd000 x20: ffff0000091d9000
>>>> x19: ffff807ccb239db0 x18: ffffffffffffffff
>>>> x17: 000000000000000e x16: 0000000000000007
>>>> x15: ffff807c5e15bd78 x14: 0000000000000000
>>>> x13: 0000000000000000 x12: 0000000000000000
>>>> x11: 0000000000000000 x10: 0000000000000001
>>>> x9 : 0000000000000228 x8 : 000000000000000c
>>>> x7 : 0000000000000fff x6 : ffff807a308ed6b0
>>>> x5 : ffff7e01f10967c0 x4 : 0000000000000018
>>>> x3 : d0bc661572445600 x2 : 0000000000000000
>>>> x1 : 000000001b2e0200 x0 : 0000000000000000
>>>> Call trace:
>>>> ocfs2_write_end_nolock+0x2b8/0x550 [ocfs2]
>>>> ocfs2_write_end+0x4c/0x80 [ocfs2]
>>>> generic_perform_write+0x108/0x1a8
>>>> __generic_file_write_iter+0x158/0x1c8
>>>> ocfs2_file_write_iter+0x668/0x950 [ocfs2]
>>>> __vfs_write+0x11c/0x190
>>>> vfs_write+0xac/0x1c0
>>>> ksys_write+0x6c/0xd8
>>>> __arm64_sys_write+0x24/0x30
>>>> el0_svc_common+0x78/0x130
>>>> el0_svc_handler+0x38/0x78
>>>> el0_svc+0x8/0xc
>>>>
>>>> To prevent NULL pointer dereference in this situation, we use
>>>> is_handle_aborted() before using handle->h_transaction->t_tid.
>>>>
>>>> Signed-off-by: Yan Wang <wangyan122@huawei.com>
>>>> Reviewed-by: Jun Piao <piaojun@huawei.com>
>>>> ---
>>>> fs/ocfs2/journal.h | 8 +++++---
>>>> fs/ocfs2/namei.c | 3 +--
>>>> 2 files changed, 6 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/fs/ocfs2/journal.h b/fs/ocfs2/journal.h
>>>> index 3103ba7f97a2..bfe611ed1b1d 100644
>>>> --- a/fs/ocfs2/journal.h
>>>> +++ b/fs/ocfs2/journal.h
>>>> @@ -597,9 +597,11 @@ static inline void ocfs2_update_inode_fsync_trans(handle_t *handle,
>>>> {
>>>> struct ocfs2_inode_info *oi = OCFS2_I(inode);
>>>>
>>>> - oi->i_sync_tid = handle->h_transaction->t_tid;
>>>> - if (datasync)
>>>> - oi->i_datasync_tid = handle->h_transaction->t_tid;
>>>> + if (!is_handle_aborted(handle)) {
>>>> + oi->i_sync_tid = handle->h_transaction->t_tid;
>>>> + if (datasync)
>>>> + oi->i_datasync_tid = handle->h_transaction->t_tid;
>>>
>>> Use tab instead of space, please.
>> Thanks, I will resend it later.
>>>
>>>> + }
>>>> }
>>>>
>>>> #endif /* OCFS2_JOURNAL_H */
>>>> diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
>>>> index 8ea51cf27b97..da65251ef815 100644
>>>> --- a/fs/ocfs2/namei.c
>>>> +++ b/fs/ocfs2/namei.c
>>>> @@ -586,8 +586,7 @@ static int __ocfs2_mknod_locked(struct inode *dir,
>>>> mlog_errno(status);
>>>> }
>>>>
>>>> - oi->i_sync_tid = handle->h_transaction->t_tid;
>>>> - oi->i_datasync_tid = handle->h_transaction->t_tid;
>>>> + ocfs2_update_inode_fsync_trans(handle, inode, 1);
>>>>
>>> I don't see any reason why we have to check handle here.
>>>
>>> Thanks,
>>> Joseph
>> Yes, it doesn't need to check handle here.
>> handle->h_transaction can not be NULL here. But?other
>> functions use ocfs2_update_inode_fsync_trans() to access
>> t_tid in handle->h_transaction.
>> I modify it for the uniform format.
>>
> I suggest we split the above cleanup into another patch. Since it
> doesn't have relations with the subject.
>
> Thanks,
> Joseph
>
OK, I will split it into another patch and resend it.
Thanks,
Yan Wang
>
>
> .
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: fix a NULL pointer dereference when call ocfs2_update_inode_fsync_trans()
2020-01-08 9:23 [Ocfs2-devel] [PATCH] ocfs2: fix a NULL pointer dereference when call ocfs2_update_inode_fsync_trans() wangyan
2020-01-08 11:31 ` Joseph Qi
@ 2020-01-09 1:57 ` Changwei Ge
2020-01-09 6:26 ` wangyan
1 sibling, 1 reply; 7+ messages in thread
From: Changwei Ge @ 2020-01-09 1:57 UTC (permalink / raw)
To: ocfs2-devel
On 1/8/20 5:23 PM, wangyan wrote:
> I found a NULL pointer dereference in ocfs2_update_inode_fsync_trans(),
> handle->h_transaction may be NULL in this situation:
> ocfs2_file_write_iter
> ->__generic_file_write_iter
> ->generic_perform_write
> ->ocfs2_write_begin
> ->ocfs2_write_begin_nolock
> ->ocfs2_write_cluster_by_desc
> ->ocfs2_write_cluster
> ->ocfs2_mark_extent_written
> ->ocfs2_change_extent_flag
> ->ocfs2_split_extent
> ->ocfs2_try_to_merge_extent
> ->ocfs2_extend_rotate_transaction
> ->ocfs2_extend_trans
> ->jbd2_journal_restart
> ->jbd2__journal_restart
> // handle->h_transaction is NULL here
> ->handle->h_transaction = NULL;
> ->start_this_handle
> /* journal aborted due to storage
> network disconnection, return error */
> ->return -EROFS;
> /* line 3806 in ocfs2_try_to_merge_extent (),
> it will ignore ret error. */
> ->ret = 0;
> ->...
> ->ocfs2_write_end
> ->ocfs2_write_end_nolock
> ->ocfs2_update_inode_fsync_trans
> // NULL pointer dereference
> ->oi->i_sync_tid = handle->h_transaction->t_tid;
>
> The information of NULL pointer dereference as follows:
> JBD2: Detected IO errors while flushing file data on dm-11-45
> Aborting journal on device dm-11-45.
> JBD2: Error -5 detected when updating journal superblock for dm-11-45.
> (dd,22081,3):ocfs2_extend_trans:474 ERROR: status = -30
> (dd,22081,3):ocfs2_try_to_merge_extent:3877 ERROR: status = -30
> Unable to handle kernel NULL pointer dereference at
> virtual address 0000000000000008
> Mem abort info:
> ESR = 0x96000004
> Exception class = DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> Data abort info:
> ISV = 0, ISS = 0x00000004
> CM = 0, WnR = 0
> user pgtable: 4k pages, 48-bit VAs, pgdp = 00000000e74e1338
> [0000000000000008] pgd=0000000000000000
> Internal error: Oops: 96000004 [#1] SMP
> Process dd (pid: 22081, stack limit = 0x00000000584f35a9)
> CPU: 3 PID: 22081 Comm: dd Kdump: loaded
> Hardware name: Huawei TaiShan 2280 V2/BC82AMDD, BIOS 0.98 08/25/2019
> pstate: 60400009 (nZCv daif +PAN -UAO)
> pc : ocfs2_write_end_nolock+0x2b8/0x550 [ocfs2]
> lr : ocfs2_write_end_nolock+0x2a0/0x550 [ocfs2]
> sp : ffff0000459fba70
> x29: ffff0000459fba70 x28: 0000000000000000
> x27: ffff807ccf7f1000 x26: 0000000000000001
> x25: ffff807bdff57970 x24: ffff807caf1d4000
> x23: ffff807cc79e9000 x22: 0000000000001000
> x21: 000000006c6cd000 x20: ffff0000091d9000
> x19: ffff807ccb239db0 x18: ffffffffffffffff
> x17: 000000000000000e x16: 0000000000000007
> x15: ffff807c5e15bd78 x14: 0000000000000000
> x13: 0000000000000000 x12: 0000000000000000
> x11: 0000000000000000 x10: 0000000000000001
> x9 : 0000000000000228 x8 : 000000000000000c
> x7 : 0000000000000fff x6 : ffff807a308ed6b0
> x5 : ffff7e01f10967c0 x4 : 0000000000000018
> x3 : d0bc661572445600 x2 : 0000000000000000
> x1 : 000000001b2e0200 x0 : 0000000000000000
> Call trace:
> ocfs2_write_end_nolock+0x2b8/0x550 [ocfs2]
> ocfs2_write_end+0x4c/0x80 [ocfs2]
> generic_perform_write+0x108/0x1a8
> __generic_file_write_iter+0x158/0x1c8
> ocfs2_file_write_iter+0x668/0x950 [ocfs2]
> __vfs_write+0x11c/0x190
> vfs_write+0xac/0x1c0
> ksys_write+0x6c/0xd8
> __arm64_sys_write+0x24/0x30
> el0_svc_common+0x78/0x130
> el0_svc_handler+0x38/0x78
> el0_svc+0x8/0xc
>
> To prevent NULL pointer dereference in this situation, we use
> is_handle_aborted() before using handle->h_transaction->t_tid.
>
> Signed-off-by: Yan Wang <wangyan122@huawei.com>
> Reviewed-by: Jun Piao <piaojun@huawei.com>
> ---
> fs/ocfs2/journal.h | 8 +++++---
> fs/ocfs2/namei.c | 3 +--
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ocfs2/journal.h b/fs/ocfs2/journal.h
> index 3103ba7f97a2..bfe611ed1b1d 100644
> --- a/fs/ocfs2/journal.h
> +++ b/fs/ocfs2/journal.h
> @@ -597,9 +597,11 @@ static inline void
> ocfs2_update_inode_fsync_trans(handle_t *handle,
> {
> struct ocfs2_inode_info *oi = OCFS2_I(inode);
>
> - oi->i_sync_tid = handle->h_transaction->t_tid;
> - if (datasync)
> - oi->i_datasync_tid = handle->h_transaction->t_tid;
> + if (!is_handle_aborted(handle)) {
> + oi->i_sync_tid = handle->h_transaction->t_tid;
> + if (datasync)
> + oi->i_datasync_tid = handle->h_transaction->t_tid;
> + }
I don't think your way can fix the issue you reported completely.
Even you check if the journal is ABORTED or not, you still face a race
causing accessing NULL h_transaction.
Otherwise, you need synchronization mechanism help.
Besides, if journal is aborted, ocfs2 won't fence the machine by resetting?
Thanks,
Changwei
> }
>
> #endif /* OCFS2_JOURNAL_H */
> diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
> index 8ea51cf27b97..da65251ef815 100644
> --- a/fs/ocfs2/namei.c
> +++ b/fs/ocfs2/namei.c
> @@ -586,8 +586,7 @@ static int __ocfs2_mknod_locked(struct inode *dir,
> mlog_errno(status);
> }
>
> - oi->i_sync_tid = handle->h_transaction->t_tid;
> - oi->i_datasync_tid = handle->h_transaction->t_tid;
> + ocfs2_update_inode_fsync_trans(handle, inode, 1);
>
> leave:
> if (status < 0) {
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: fix a NULL pointer dereference when call ocfs2_update_inode_fsync_trans()
2020-01-09 1:57 ` Changwei Ge
@ 2020-01-09 6:26 ` wangyan
0 siblings, 0 replies; 7+ messages in thread
From: wangyan @ 2020-01-09 6:26 UTC (permalink / raw)
To: ocfs2-devel
On 2020/1/9 9:57, Changwei Ge wrote:
>
> On 1/8/20 5:23 PM, wangyan wrote:
>> I found a NULL pointer dereference in ocfs2_update_inode_fsync_trans(),
>> handle->h_transaction may be NULL in this situation:
>> ocfs2_file_write_iter
>> ->__generic_file_write_iter
>> ->generic_perform_write
>> ->ocfs2_write_begin
>> ->ocfs2_write_begin_nolock
>> ->ocfs2_write_cluster_by_desc
>> ->ocfs2_write_cluster
>> ->ocfs2_mark_extent_written
>> ->ocfs2_change_extent_flag
>> ->ocfs2_split_extent
>> ->ocfs2_try_to_merge_extent
>> ->ocfs2_extend_rotate_transaction
>> ->ocfs2_extend_trans
>> ->jbd2_journal_restart
>> ->jbd2__journal_restart
>> // handle->h_transaction is NULL here
>> ->handle->h_transaction = NULL;
>> ->start_this_handle
>> /* journal aborted due to storage
>> network disconnection, return error */
>> ->return -EROFS;
>> /* line 3806 in ocfs2_try_to_merge_extent (),
>> it will ignore ret error. */
>> ->ret = 0;
>> ->...
>> ->ocfs2_write_end
>> ->ocfs2_write_end_nolock
>> ->ocfs2_update_inode_fsync_trans
>> // NULL pointer dereference
>> ->oi->i_sync_tid = handle->h_transaction->t_tid;
>>
>> The information of NULL pointer dereference as follows:
>> JBD2: Detected IO errors while flushing file data on dm-11-45
>> Aborting journal on device dm-11-45.
>> JBD2: Error -5 detected when updating journal superblock for dm-11-45.
>> (dd,22081,3):ocfs2_extend_trans:474 ERROR: status = -30
>> (dd,22081,3):ocfs2_try_to_merge_extent:3877 ERROR: status = -30
>> Unable to handle kernel NULL pointer dereference at
>> virtual address 0000000000000008
>> Mem abort info:
>> ESR = 0x96000004
>> Exception class = DABT (current EL), IL = 32 bits
>> SET = 0, FnV = 0
>> EA = 0, S1PTW = 0
>> Data abort info:
>> ISV = 0, ISS = 0x00000004
>> CM = 0, WnR = 0
>> user pgtable: 4k pages, 48-bit VAs, pgdp = 00000000e74e1338
>> [0000000000000008] pgd=0000000000000000
>> Internal error: Oops: 96000004 [#1] SMP
>> Process dd (pid: 22081, stack limit = 0x00000000584f35a9)
>> CPU: 3 PID: 22081 Comm: dd Kdump: loaded
>> Hardware name: Huawei TaiShan 2280 V2/BC82AMDD, BIOS 0.98 08/25/2019
>> pstate: 60400009 (nZCv daif +PAN -UAO)
>> pc : ocfs2_write_end_nolock+0x2b8/0x550 [ocfs2]
>> lr : ocfs2_write_end_nolock+0x2a0/0x550 [ocfs2]
>> sp : ffff0000459fba70
>> x29: ffff0000459fba70 x28: 0000000000000000
>> x27: ffff807ccf7f1000 x26: 0000000000000001
>> x25: ffff807bdff57970 x24: ffff807caf1d4000
>> x23: ffff807cc79e9000 x22: 0000000000001000
>> x21: 000000006c6cd000 x20: ffff0000091d9000
>> x19: ffff807ccb239db0 x18: ffffffffffffffff
>> x17: 000000000000000e x16: 0000000000000007
>> x15: ffff807c5e15bd78 x14: 0000000000000000
>> x13: 0000000000000000 x12: 0000000000000000
>> x11: 0000000000000000 x10: 0000000000000001
>> x9 : 0000000000000228 x8 : 000000000000000c
>> x7 : 0000000000000fff x6 : ffff807a308ed6b0
>> x5 : ffff7e01f10967c0 x4 : 0000000000000018
>> x3 : d0bc661572445600 x2 : 0000000000000000
>> x1 : 000000001b2e0200 x0 : 0000000000000000
>> Call trace:
>> ocfs2_write_end_nolock+0x2b8/0x550 [ocfs2]
>> ocfs2_write_end+0x4c/0x80 [ocfs2]
>> generic_perform_write+0x108/0x1a8
>> __generic_file_write_iter+0x158/0x1c8
>> ocfs2_file_write_iter+0x668/0x950 [ocfs2]
>> __vfs_write+0x11c/0x190
>> vfs_write+0xac/0x1c0
>> ksys_write+0x6c/0xd8
>> __arm64_sys_write+0x24/0x30
>> el0_svc_common+0x78/0x130
>> el0_svc_handler+0x38/0x78
>> el0_svc+0x8/0xc
>>
>> To prevent NULL pointer dereference in this situation, we use
>> is_handle_aborted() before using handle->h_transaction->t_tid.
>>
>> Signed-off-by: Yan Wang <wangyan122@huawei.com>
>> Reviewed-by: Jun Piao <piaojun@huawei.com>
>> ---
>> fs/ocfs2/journal.h | 8 +++++---
>> fs/ocfs2/namei.c | 3 +--
>> 2 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/ocfs2/journal.h b/fs/ocfs2/journal.h
>> index 3103ba7f97a2..bfe611ed1b1d 100644
>> --- a/fs/ocfs2/journal.h
>> +++ b/fs/ocfs2/journal.h
>> @@ -597,9 +597,11 @@ static inline void
>> ocfs2_update_inode_fsync_trans(handle_t *handle,
>> {
>> struct ocfs2_inode_info *oi = OCFS2_I(inode);
>>
>> - oi->i_sync_tid = handle->h_transaction->t_tid;
>> - if (datasync)
>> - oi->i_datasync_tid = handle->h_transaction->t_tid;
>> + if (!is_handle_aborted(handle)) {
>> + oi->i_sync_tid = handle->h_transaction->t_tid;
>> + if (datasync)
>> + oi->i_datasync_tid = handle->h_transaction->t_tid;
>> + }
>
>
> I don't think your way can fix the issue you reported completely.
>
> Even you check if the journal is ABORTED or not, you still face a race
> causing accessing NULL h_transaction.
>
> Otherwise, you need synchronization mechanism help.
Just one process, and handle is not shared by other processes, no race here.
>
> Besides, if journal is aborted, ocfs2 won't fence the machine by resetting?
Even journal is not aborted, it still has other wrong branch in
start_this_handle(), which will return error before assign value to
handle->h_transaction.
>
>
> Thanks,
>
> Changwei
>
>
>> }
>>
>> #endif /* OCFS2_JOURNAL_H */
>> diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
>> index 8ea51cf27b97..da65251ef815 100644
>> --- a/fs/ocfs2/namei.c
>> +++ b/fs/ocfs2/namei.c
>> @@ -586,8 +586,7 @@ static int __ocfs2_mknod_locked(struct inode *dir,
>> mlog_errno(status);
>> }
>>
>> - oi->i_sync_tid = handle->h_transaction->t_tid;
>> - oi->i_datasync_tid = handle->h_transaction->t_tid;
>> + ocfs2_update_inode_fsync_trans(handle, inode, 1);
>>
>> leave:
>> if (status < 0) {
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-01-09 6:26 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-01-08 9:23 [Ocfs2-devel] [PATCH] ocfs2: fix a NULL pointer dereference when call ocfs2_update_inode_fsync_trans() wangyan
2020-01-08 11:31 ` Joseph Qi
2020-01-08 12:14 ` wangyan
2020-01-09 1:25 ` Joseph Qi
2020-01-09 1:37 ` wangyan
2020-01-09 1:57 ` Changwei Ge
2020-01-09 6:26 ` wangyan
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.