linux-kernel - Re: [PATCH v3 1/6] vfio/type1: Introduce iova list and add iommu aperture validity check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 19 Feb 2018 12:51:13 -0700
From:   Alex Williamson <alex.williamson@...hat.com>
To:     Shameerali Kolothum Thodi <shameerali.kolothum.thodi@...wei.com>
Cc:     "eric.auger@...hat.com" <eric.auger@...hat.com>,
        "pmorel@...ux.vnet.ibm.com" <pmorel@...ux.vnet.ibm.com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linuxarm <linuxarm@...wei.com>,
        John Garry <john.garry@...wei.com>,
        "xuwei (O)" <xuwei5@...wei.com>
Subject: Re: [PATCH v3 1/6] vfio/type1: Introduce iova list and add iommu
 aperture validity check

On Mon, 19 Feb 2018 09:50:24 +0000
Shameerali Kolothum Thodi <shameerali.kolothum.thodi@...wei.com> wrote:
> > -----Original Message-----
> > From: Alex Williamson [mailto:alex.williamson@...hat.com]
> > Sent: Friday, February 16, 2018 8:49 PM 
> > On Thu, 15 Feb 2018 09:44:59 +0000
> > Shameer Kolothum <shameerali.kolothum.thodi@...wei.com> wrote:
> > > +			node->end = end;
> > > +			continue;
> > > +		}
> > > +		/* Delete nodes after new end */
> > > +		list_del(&node->list);
> > > +		kfree(node);
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int vfio_iommu_get_iova_copy(struct vfio_iommu *iommu,
> > > +				struct list_head *iova_copy)
> > > +{
> > > +
> > > +	struct list_head *iova = &iommu->iova_list;
> > > +	struct vfio_iova *n;
> > > +
> > > +	list_for_each_entry(n, iova, list) {
> > > +		int ret;
> > > +
> > > +		ret = vfio_insert_iova(n->start, n->end, iova_copy);
> > > +		if (ret)
> > > +			return ret;  
> > 
> > Let's delete and free any entries added to the copy here too.  
> 
> Ok. My original thought was caller will free up in case of error.

This comes down to Rusty's suggestions of how to make an API hard to
misuse rather than simply easy to use to me.  Placing the onus on the
caller to cleanup a list sounds simple, but the caller passed an empty
list and the function failed, why should the caller bother to check if
the function left any cruft on the list in the course of failing?  This
is not a hard to misuse interface, in fact it's very easy to forget
that cleanup.  Thanks,

Alex