Re: [PATCH v4] misc: pci_endpoint_test: Use memcpy_toio()/memcpy_fromio() for BAR tests

[Niklas Cassel <cassel@kernel.org>]

Hello Kuppuswamy, Dan,

On Fri, Mar 22, 2024 at 10:03:12AM -0700, Kuppuswamy Sathyanarayanan wrote:
> > +
> >  static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
> >  				  enum pci_barno barno)
> >  {
> > -	int j;
> > -	u32 val;
> > -	int size;
> > +	int j, bar_size, buf_size, iters, remain;
> > +	void *write_buf __free(kfree) = NULL;
> > +	void *read_buf __free(kfree) = NULL;
> 
> Please check the following cleanup doc. Recommendation is to avoid above __free(kfree) = NULL declarations and directly define write_buf/read_buf.
> 
> https://lore.kernel.org/lkml/171097196970.1011049.9726486429680041876.stgit@dwillia2-xfh.jf.intel.com/T/#m05d71a668ff0fad46c2055dbdcde55d7381780b7

I don't think that the docs say that using:
void *ptr __free(kfree) = NULL;
is always an anti-pattern.

"If other cleanup helpers appeared in such a function that depended on
@dev being live to complete their unwind then using the
"struct obj_type *obj __free(...) = NULL" style is an anti-pattern that
potentially causes a use-after-free bug."

I think it says that it is an anti-pattern IFF there are two cleanup
helpers in a function AND they have have a inter-dependency.

This patch just adds a single cleanup helper, so there can be no
inter-dependency problem.

This pattern is common in Linus's current tree, see e.g.
$ git grep -C 3 -E "__free(.*) = NULL"

If this was a problem, I don't think we would have seen
100 different instances of this pattern.

In other words, I think this patch is fine.

Dan, please correct me if I'm wrong.


Kind regards,
Niklas