xfs: don't wrap ID in xfs_dq_get_next_id
diff mbox

Message ID f52de68d-abe6-1960-c0ef-1d199346f689@redhat.com
State Accepted
Headers show

Commit Message

Eric Sandeen Dec. 17, 2016, 12:05 a.m. UTC
The GETNEXTQOTA ioctl takes whatever ID is sent in,
and looks for the next active quota for an user
equal or higher to that ID.

But if we are at the maximum ID and then ask for the "next"
one, we may wrap back to zero.  In this case, userspace
may loop forever, because it will start querying again
at zero.

We'll fix this in userspace as well, but for the kernel,
return -ENOENT if we ask for the next quota ID
past UINT_MAX so the caller knows to stop.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---


--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Christoph Hellwig Dec. 19, 2016, 10:26 a.m. UTC | #1
On Fri, Dec 16, 2016 at 06:05:20PM -0600, Eric Sandeen wrote:
> The GETNEXTQOTA ioctl takes whatever ID is sent in,
> and looks for the next active quota for an user
> equal or higher to that ID.
> 
> But if we are at the maximum ID and then ask for the "next"
> one, we may wrap back to zero.  In this case, userspace
> may loop forever, because it will start querying again
> at zero.
> 
> We'll fix this in userspace as well, but for the kernel,
> return -ENOENT if we ask for the next quota ID
> past UINT_MAX so the caller knows to stop.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Sandeen Jan. 13, 2017, 10:48 p.m. UTC | #2
On 12/16/16 6:05 PM, Eric Sandeen wrote:
> The GETNEXTQOTA ioctl takes whatever ID is sent in,
> and looks for the next active quota for an user
> equal or higher to that ID.
> 
> But if we are at the maximum ID and then ask for the "next"
> one, we may wrap back to zero.  In this case, userspace
> may loop forever, because it will start querying again
> at zero.
> 
> We'll fix this in userspace as well, but for the kernel,
> return -ENOENT if we ask for the next quota ID
> past UINT_MAX so the caller knows to stop.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Ping on this?  sorry for not noticing its absence sooner.
Has rvb:hch on the thread.

-Eric

> ---
> 
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 7a30b8f..dbeddd9 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -710,6 +710,10 @@
>  	/* Simple advance */
>  	next_id = *id + 1;
>  
> +	/* If we'd wrap past the max ID, stop */
> +	if (next_id < *id)
> +		return -ENOENT;
> +
>  	/* If new ID is within the current chunk, advancing it sufficed */
>  	if (next_id % mp->m_quotainfo->qi_dqperchunk) {
>  		*id = next_id;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 7a30b8f..dbeddd9 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -710,6 +710,10 @@ 
 	/* Simple advance */
 	next_id = *id + 1;
 
+	/* If we'd wrap past the max ID, stop */
+	if (next_id < *id)
+		return -ENOENT;
+
 	/* If new ID is within the current chunk, advancing it sufficed */
 	if (next_id % mp->m_quotainfo->qi_dqperchunk) {
 		*id = next_id;