| Message ID | 20230407130033.556644-1-mlombard@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a0fde512f7030e7fbdd5f0d8d70908d37389aae2 |
| Headers | show |
| Series | target: core: fix invalid memory access | expand |
ACK
On 4/7/23 8:00 AM, Maurizio Lombardi wrote: > nr_attrs should start counting from zero, otherwise we > will end up dereferencing an invalid memory address. > > $ targetcli /loopback create > > general protection fault > RIP: 0010:configfs_create_file+0x12/0x70 > Call Trace: > <TASK> > configfs_attach_item.part.0+0x5f/0x150 > configfs_attach_group.isra.0+0x49/0x120 > configfs_mkdir+0x24f/0x4d0 > vfs_mkdir+0x192/0x240 > do_mkdirat+0x131/0x160 > __x64_sys_mkdir+0x48/0x70 > do_syscall_64+0x5c/0x90 > > Fixes: 31177b74790c ("scsi: target: core: Add RTPI attribute for target port") > > Signed-off-by: Maurizio Lombardi <mlombard@redhat.com> > --- > drivers/target/target_core_fabric_configfs.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c > index 0ce47e21e0c8..b7c637644cd4 100644 > --- a/drivers/target/target_core_fabric_configfs.c > +++ b/drivers/target/target_core_fabric_configfs.c > @@ -885,7 +885,7 @@ target_fabric_setup_tpg_base_cit(struct target_fabric_configfs *tf) > { > struct config_item_type *cit = &tf->tf_tpg_base_cit; > struct configfs_attribute **attrs = NULL; > - size_t nr_attrs = 1; > + size_t nr_attrs = 0; > int i = 0; > > if (tf->tf_ops->tfc_tpg_base_attrs) > @@ -895,8 +895,8 @@ target_fabric_setup_tpg_base_cit(struct target_fabric_configfs *tf) > if (tf->tf_ops->fabric_enable_tpg) > nr_attrs++; > > - if (nr_attrs == 0) > - goto done; > + /* + 1 for target_fabric_tpg_base_attr_rtpi */ > + nr_attrs++; > > /* + 1 for final NULL in the array */ > attrs = kcalloc(nr_attrs + 1, sizeof(*attrs), GFP_KERNEL); > @@ -912,7 +912,6 @@ target_fabric_setup_tpg_base_cit(struct target_fabric_configfs *tf) > > attrs[i++] = &target_fabric_tpg_base_attr_rtpi; > > -done: > cit->ct_item_ops = &target_fabric_tpg_base_item_ops; > cit->ct_attrs = attrs; > cit->ct_owner = tf->tf_ops->module; Reviewed-by: Mike Christie <michael.christie@oracle.com>
Maurizio, > nr_attrs should start counting from zero, otherwise we will end up > dereferencing an invalid memory address. Applied to 6.4/scsi-staging, thanks!
diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c index 0ce47e21e0c8..b7c637644cd4 100644 --- a/drivers/target/target_core_fabric_configfs.c +++ b/drivers/target/target_core_fabric_configfs.c @@ -885,7 +885,7 @@ target_fabric_setup_tpg_base_cit(struct target_fabric_configfs *tf) { struct config_item_type *cit = &tf->tf_tpg_base_cit; struct configfs_attribute **attrs = NULL; - size_t nr_attrs = 1; + size_t nr_attrs = 0; int i = 0; if (tf->tf_ops->tfc_tpg_base_attrs) @@ -895,8 +895,8 @@ target_fabric_setup_tpg_base_cit(struct target_fabric_configfs *tf) if (tf->tf_ops->fabric_enable_tpg) nr_attrs++; - if (nr_attrs == 0) - goto done; + /* + 1 for target_fabric_tpg_base_attr_rtpi */ + nr_attrs++; /* + 1 for final NULL in the array */ attrs = kcalloc(nr_attrs + 1, sizeof(*attrs), GFP_KERNEL); @@ -912,7 +912,6 @@ target_fabric_setup_tpg_base_cit(struct target_fabric_configfs *tf) attrs[i++] = &target_fabric_tpg_base_attr_rtpi; -done: cit->ct_item_ops = &target_fabric_tpg_base_item_ops; cit->ct_attrs = attrs; cit->ct_owner = tf->tf_ops->module;
nr_attrs should start counting from zero, otherwise we will end up dereferencing an invalid memory address. $ targetcli /loopback create general protection fault RIP: 0010:configfs_create_file+0x12/0x70 Call Trace: <TASK> configfs_attach_item.part.0+0x5f/0x150 configfs_attach_group.isra.0+0x49/0x120 configfs_mkdir+0x24f/0x4d0 vfs_mkdir+0x192/0x240 do_mkdirat+0x131/0x160 __x64_sys_mkdir+0x48/0x70 do_syscall_64+0x5c/0x90 Fixes: 31177b74790c ("scsi: target: core: Add RTPI attribute for target port") Signed-off-by: Maurizio Lombardi <mlombard@redhat.com> --- drivers/target/target_core_fabric_configfs.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)