All of lore.kernel.org
 help / color / mirror / Atom feed
From: Niklas Schnelle <schnelle@linux.ibm.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Will Deacon <will@kernel.org>,
	linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
	Jason Gunthorpe <jgg@nvidia.com>,
	Robin Murphy <robin.murphy@arm.com>
Subject: Re: [PATCH] iommu/dma: Fix check for error return from iommu_map_sg_atomic()
Date: Tue, 17 May 2022 12:17:29 +0200	[thread overview]
Message-ID: <27ae8b9f8e61dce4b31a37622e98b1c57b21b104.camel@linux.ibm.com> (raw)
In-Reply-To: <20220517083657.GA16377@lst.de>

On Tue, 2022-05-17 at 10:36 +0200, Christoph Hellwig wrote:
> On Fri, May 13, 2022 at 05:39:48PM +0200, Niklas Schnelle wrote:
> > In __iommu_dma_alloc_noncontiguous() the value returned by
> > iommu_map_sg_atomic() is checked for being smaller than size. Before
> > commit ad8f36e4b6b1 ("iommu: return full error code from
> > iommu_map_sg[_atomic]()") this simply checked if the requested size was
> > successfully mapped.
> > 
> > After that commit iommu_map_sg_atomic() may also return a negative
> > error value. In principle this too would be covered by the existing
> > check. There is one problem however, as size is of type size_t while the
> > return type of iommu_map_sg_atomic() is now of type ssize_t the latter gets
> > converted to size_t and negative error values end up as very large
> > positive values making the check succeed. Fix this by making the return
> > type visible with a local variable and add an explicit cast to ssize_t.
> > 
> > Fixes: ad8f36e4b6b1 ("iommu: return full error code from iommu_map_sg[_atomic]()")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> 
> I don't see what the point of the newly added local variable is here.
> Just casting size should be all that is needed as far as I can tell.

No technical reason just found it easier to read and more descriptive.
I'll sent a v2 with just the cast, it does simplify the commit message.

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

WARNING: multiple messages have this Message-ID (diff)
From: Niklas Schnelle <schnelle@linux.ibm.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>,
	Jason Gunthorpe <jgg@nvidia.com>,
	Robin Murphy <robin.murphy@arm.com>,
	Matthew Rosato <mjrosato@linux.ibm.com>,
	linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org
Subject: Re: [PATCH] iommu/dma: Fix check for error return from iommu_map_sg_atomic()
Date: Tue, 17 May 2022 12:17:29 +0200	[thread overview]
Message-ID: <27ae8b9f8e61dce4b31a37622e98b1c57b21b104.camel@linux.ibm.com> (raw)
In-Reply-To: <20220517083657.GA16377@lst.de>

On Tue, 2022-05-17 at 10:36 +0200, Christoph Hellwig wrote:
> On Fri, May 13, 2022 at 05:39:48PM +0200, Niklas Schnelle wrote:
> > In __iommu_dma_alloc_noncontiguous() the value returned by
> > iommu_map_sg_atomic() is checked for being smaller than size. Before
> > commit ad8f36e4b6b1 ("iommu: return full error code from
> > iommu_map_sg[_atomic]()") this simply checked if the requested size was
> > successfully mapped.
> > 
> > After that commit iommu_map_sg_atomic() may also return a negative
> > error value. In principle this too would be covered by the existing
> > check. There is one problem however, as size is of type size_t while the
> > return type of iommu_map_sg_atomic() is now of type ssize_t the latter gets
> > converted to size_t and negative error values end up as very large
> > positive values making the check succeed. Fix this by making the return
> > type visible with a local variable and add an explicit cast to ssize_t.
> > 
> > Fixes: ad8f36e4b6b1 ("iommu: return full error code from iommu_map_sg[_atomic]()")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> 
> I don't see what the point of the newly added local variable is here.
> Just casting size should be all that is needed as far as I can tell.

No technical reason just found it easier to read and more descriptive.
I'll sent a v2 with just the cast, it does simplify the commit message.


  reply	other threads:[~2022-05-17 10:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-13 15:39 [PATCH] iommu/dma: Fix check for error return from iommu_map_sg_atomic() Niklas Schnelle
2022-05-13 15:39 ` Niklas Schnelle
2022-05-17  8:36 ` Christoph Hellwig
2022-05-17  8:36   ` Christoph Hellwig
2022-05-17 10:17   ` Niklas Schnelle [this message]
2022-05-17 10:17     ` Niklas Schnelle
2022-05-17 10:18     ` Robin Murphy
2022-05-17 10:18       ` Robin Murphy
2022-05-17 10:32       ` Niklas Schnelle
2022-05-17 10:32         ` Niklas Schnelle

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=27ae8b9f8e61dce4b31a37622e98b1c57b21b104.camel@linux.ibm.com \
    --to=schnelle@linux.ibm.com \
    --cc=hch@lst.de \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jgg@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.