Re: [PATCH v2] scsi: target: copy iSCSI ISID before unmapping the PR OUT buffer

[John Garry]

On 09/06/2026 12:36, James Bottomley wrote:
> On Tue, 2026-06-09 at 09:50 +0100, John Garry wrote:
> > > @@ -1808,9 +1809,11 @@ core_scsi3_decode_spec_i_port(
> > >     		core_scsi3_tpg_undepend_item(dest_tpg);
> > >     	}
> > >
> > > +	kfree(iport_ptr);
> > >     	return 0;
> > >     out_unmap:
> > >     	transport_kunmap_data_sg(cmd);
> > > +	kfree(iport_ptr);
> >
> > sorry for suggesting this change, but this is worse than what you had
> > originally, as we have so many paths to call kfree() [which means
> > more bugs possible]
> >
> > it's hard to make good suggestions for this target code as the
> > functions are so large and complex.
>
> Given that it's allocated in a function called by the routine but never
> actually retained by anything what about defining it as
>
> chat *iport_ptr __free(kfree) = NULL;
>
> ?  That way we don't need to care about freeing it in the error legs.

It's not just freed in the error legs, as 
core_scsi3_decode_spec_i_port() calls target_parse_pr_out() in a loop

while (tpdl > 0) {

...

	spin_lock(&dev->se_port_lock);
	list_for_each_entry(tmp_lun, &dev->dev_sep_list, lun_dev_link) {
		...

		kfree(iport_ptr);
		iport_ptr = NULL;
		tid_found = target_parse_pr_out_transport_id(tmp_tpg,
				ptr, &tid_len, &iport_ptr, i_str);
		if (!tid_found)
			continue;

....

	kfree(iport_ptr);
out_unmap:
	kfree(iport_ptr);
out:
	...

If __free(kfree) attr can still handle this sort of flow, then ok.

I still think that it would be simpler to keep the mapping in place for 
longer (to avoid all of this), if possible.

Thanks,
John