Re: [PATCH 2/2] target: core: de-RCU of se_lun and se_lun acl

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/20/22 6:28 AM, Dmitry Bogdanov wrote:
> @@ -1437,34 +1435,26 @@ static void core_scsi3_nodeacl_undepend_item(struct se_node_acl *nacl)
>  
>  static int core_scsi3_lunacl_depend_item(struct se_dev_entry *se_deve)
>  {
> -	struct se_lun_acl *lun_acl;
> -
>  	/*
>  	 * For nacl->dynamic_node_acl=1
>  	 */
> -	lun_acl = rcu_dereference_check(se_deve->se_lun_acl,
> -				kref_read(&se_deve->pr_kref) != 0);
> -	if (!lun_acl)
> +	if (!se_deve->se_lun_acl)
>  		return 0;

Will se_lun_acl and se_lun ever be NULL now? Should this maybe be a
WARN_ON or return a -EXYZ error code like when target_depend_item fails
at the bottom of the function?


I don't think it's a problem with your patch. I think we had issues
before it. If they were NULL then I think we can hit issues like:

1. __core_scsi3_alloc_registration calls core_scsi3_lunacl_depend_item.
If se_lun_acl is NULL and we return 0.

2. __core_scsi3_alloc_registration and other callers treat 0 as success.

3. It then does:

dest_lun = deve_tmp->se_lun;

pr_reg_atp = __core_scsi3_do_alloc_registration(dev,
 				nacl_tmp, dest_lun, deve_tmp,

4. So if se_lun is also NULL then for __core_scsi3_do_alloc_registration's
args we have:

dest_deve, as non-NULL
lun as NULL

5. __core_scsi3_do_alloc_registration will then do a

        pr_reg->pr_aptpl_target_lun = lun->unpacked_lun;
        pr_reg->tg_pt_sep_rtpi = lun->lun_rtpi;

and since lun is NULL we will crash.




>  
> -	return target_depend_item(&lun_acl->se_lun_group.cg_item);
> +	return target_depend_item(&se_deve->se_lun_acl->se_lun_group.cg_item);
>  }
>  
>  static void core_scsi3_lunacl_undepend_item(struct se_dev_entry *se_deve)
>  {
> -	struct se_lun_acl *lun_acl;
> -
> -	/*
> +/*

Just drop the misformatting bit above.

>  	 * For nacl->dynamic_node_acl=1
>  	 */
> -	lun_acl = rcu_dereference_check(se_deve->se_lun_acl,
> -				kref_read(&se_deve->pr_kref) != 0);
> -	if (!lun_acl) {
> +	if (!se_deve->se_lun_acl) {
>  		kref_put(&se_deve->pr_kref, target_pr_kref_release);
>  		return;
>  	}
>  
> -	target_undepend_item(&lun_acl->se_lun_group.cg_item);
> +	target_undepend_item(&se_deve->se_lun_acl->se_lun_group.cg_item);
>  	kref_put(&se_deve->pr_kref, target_pr_kref_release);
>  }



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux