[PATCH] Validate string -> number conversion.

[PATCH] Validate string -> number conversion.

[Arkadiusz Miśkiewicz]

Make sure that numbers passed as string will fit into proper
types when doing string->uid_t/gid_t/prid_t conversion.

Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl>
---
 libxcmd/input.c |   18 +++++++++++++++---
 quota/project.c |    2 +-
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/libxcmd/input.c b/libxcmd/input.c
index 1bc0745..c7807fe 100644
--- a/libxcmd/input.c
+++ b/libxcmd/input.c
@@ -337,13 +337,17 @@ prid_from_string(
 {
 	fs_project_t	*prj;
 	prid_t		prid;
+	unsigned long int	prid_long;
 	char		*sp;
 
 	/*
 	 * Allow either a full numeric or a valid projectname, even
 	 * if it starts with a digit.
 	 */
-	prid = (prid_t)strtoul(project, &sp, 10);
+	prid_long = strtoul(project, &sp, 10);
+	if ((prid_long == ULONG_MAX && errno == ERANGE) || (prid_long > (prid_t)-1))
+		return -1;
+	prid = (prid_t)prid_long;
 	if (*project != '\0' && *sp == '\0')
 		return prid;
 	prj = getprnam(project);
@@ -358,9 +362,13 @@ uid_from_string(
 {
 	struct passwd	*pwd;
 	uid_t		uid;
+	unsigned long int	uid_long;
 	char		*sp;
 
-	uid = (uid_t)strtoul(user, &sp, 10);
+	uid_long = strtoul(user, &sp, 10);
+	if ((uid_long == ULONG_MAX && errno == ERANGE) || (uid_long > (uid_t)-1))
+		return -1;
+	uid = (uid_t)uid_long;
 	if (sp != user)
 		return uid;
 	pwd = getpwnam(user);
@@ -375,9 +383,13 @@ gid_from_string(
 {
 	struct group	*grp;
 	gid_t		gid;
+	unsigned long int	gid_long;
 	char		*sp;
 
-	gid = (gid_t)strtoul(group, &sp, 10);
+	gid_long = strtoul(group, &sp, 10);
+	if ((gid_long == ULONG_MAX && errno == ERANGE) || (gid_long > (gid_t)-1))
+		return -1;
+	gid = (gid_t)gid_long;
 	if (sp != group)
 		return gid;
 	grp = getgrnam(group);
diff --git a/quota/project.c b/quota/project.c
index 1aacddd..e9baadd 100644
--- a/quota/project.c
+++ b/quota/project.c
@@ -331,7 +331,7 @@ project_f(
 		prid = prid_from_string(argv[optind]);
 		if (prid == -1) {
 			exitcode = 1;
-			fprintf(stderr, _("%s - no such project in %s\n"),
+			fprintf(stderr, _("%s - no such project in %s or invalid project number\n"),
 				argv[optind], projects_file);
 		} else
 	                project(argv[optind], type);
-- 
1.7.1.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] Validate string -> number conversion.

[Arkadiusz Miskiewicz]

On Wednesday 25 of August 2010, Arkadiusz Miśkiewicz wrote:
> Make sure that numbers passed as string will fit into proper
> types when doing string->uid_t/gid_t/prid_t conversion.
> 
> Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl>
> ---
>  libxcmd/input.c |   18 +++++++++++++++---
>  quota/project.c |    2 +-
>  2 files changed, 16 insertions(+), 4 deletions(-)

On the kernel side something like below is needed (compile tested only).
The true fix is to extend on disk di_projid to 32bit (there is room for that).

diff --git a/fs/xfs/linux-2.6/xfs_ioctl.c b/fs/xfs/linux-2.6/xfs_ioctl.c
index 237f5ff..1d9cf4f 100644
--- a/fs/xfs/linux-2.6/xfs_ioctl.c
+++ b/fs/xfs/linux-2.6/xfs_ioctl.c
@@ -906,6 +906,11 @@ xfs_ioctl_setattr(
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return XFS_ERROR(EIO);
 
+	/* Make sure a userspace passed projid (32bit) will fit on disk (16bit) */
+	if (fa->fsx_projid > (__uint16_t)-1) {
+		code = XFS_ERROR(EINVAL);                                                                                                    
+		goto error_return; 
+	}
 	/*
 	 * If disk quotas is on, we make sure that the dquots do exist on disk,
 	 * before we start any other transactions. Trying to do this later


-- 
Arkadiusz Miśkiewicz        PLD/Linux Team
arekm / maven.pl            http://ftp.pld-linux.org/

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] Validate string -> number conversion.

[Arkadiusz Miskiewicz]

On Wednesday 25 of August 2010, Arkadiusz Miskiewicz wrote:
> On Wednesday 25 of August 2010, Arkadiusz Miśkiewicz wrote:
> > Make sure that numbers passed as string will fit into proper
> > types when doing string->uid_t/gid_t/prid_t conversion.
> > 
> > Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl>
> > ---
> > 
> >  libxcmd/input.c |   18 +++++++++++++++---
> >  quota/project.c |    2 +-
> >  2 files changed, 16 insertions(+), 4 deletions(-)
> 
> On the kernel side something like below is needed (compile tested only).
> The true fix is to extend on disk di_projid to 32bit (there is room for
> that).

template of test suite for this problem
(likely needs some small amount of work)

#! /bin/bash
# FS QA Test No. xxx
#
# test to verify that correct project quota id is set
#
# creator
owner=arekm@maven.pl

seq=`basename $0`
echo "QA output created by $seq"

here=`pwd`
tmp=/tmp/$$
status=1	# failure is the default!

# get standard environment, filters and checks
. ./common.rc
. ./common.filter
. ./common.quota

_cleanup()
{
	cd /
	umount $SCRATCH_MNT 2>/dev/null
	rm -f $tmp.*
}
#trap "_cleanup; exit \$status" 0 1 2 3 15


# real QA test starts here
_supported_fs xfs
_supported_os Linux IRIX

_require_xfs_quota

dir=$SCRATCH_MNT/project

_require_scratch
_scratch_mkfs_xfs >/dev/null 2>&1

#if pquota's already in mount options then we dont need to enable

# we can't run with group quotas
if ( `echo $MOUNT_OPTIONS | grep -q gquota` || `echo $MOUNT_OPTIONS | grep -q grpquota` )
then
    _notrun "Can't run with group quotas enabled"
fi
EXTRA_MOUNT_OPTIONS="-o pquota"

if ! _scratch_mount "$EXTRA_MOUNT_OPTIONS" >$tmp.out 2>&1
then
    cat $tmp.out
    echo "!!! mount failed"
    exit
fi

src/feature -p $SCRATCH_DEV
[ $? -ne 0 ] && _notrun "Installed kernel does not support project quotas"

mkdir $dir
touch $dir/below16bit
# below 16bit value
xfs_quota -x -c "project -s -p $dir/below16bit 3422" $SCRATCH_DEV
projid=$($XFS_IO_PROG -r -c "lsproj" $dir/below16bit)
if [ "projid = 3422" != "$projid" ]; then
	echo "FAIL: returned projid value ($projid) doesn't match set one (3422)"
fi

# over 16bit value
touch $dir/over16bit
xfs_quota -x -c "project -s -p $dir/over16bit 108545" $SCRATCH_DEV
projid=$($XFS_IO_PROG -r -c "lsproj" $dir)
if [ "projid = 108545" != "$projid" ]; then
	echo "FAIL: returned projid value ($projid) doesn't match set one (108545)"
fi

# over 32bit value, should fail
touch $dir/over32bit
if ! xfs_quota -x -c "project -s -p $dir/over32bit 5344967296" $SCRATCH_DEV; then
	echo "FAIL: setting over 32bit projid succeeded while it should fail"
fi

# success, all done
status=0
exit


-- 
Arkadiusz Miśkiewicz        PLD/Linux Team
arekm / maven.pl            http://ftp.pld-linux.org/

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] Validate string -> number conversion.

[Dave Chinner]

On Wed, Aug 25, 2010 at 11:01:26AM +0200, Arkadiusz Miskiewicz wrote:
> On Wednesday 25 of August 2010, Arkadiusz Miskiewicz wrote:
> > On Wednesday 25 of August 2010, Arkadiusz Miśkiewicz wrote:
> > > Make sure that numbers passed as string will fit into proper
> > > types when doing string->uid_t/gid_t/prid_t conversion.
> > > 
> > > Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl>
> > > ---
> > > 
> > >  libxcmd/input.c |   18 +++++++++++++++---
> > >  quota/project.c |    2 +-
> > >  2 files changed, 16 insertions(+), 4 deletions(-)
> > 
> > On the kernel side something like below is needed (compile tested only).
> > The true fix is to extend on disk di_projid to 32bit (there is room for
> > that).
> 
> template of test suite for this problem
> (likely needs some small amount of work)

Great - thanks for doing this. A couple of comments, though.

First, I'm assuming that no output should occur if everything
succeeds?  If so, we normally put an:

echo "Silence is golden"

in the test to document that we expect no other output.

> 
> #! /bin/bash
> # FS QA Test No. xxx
> #
> # test to verify that correct project quota id is set
> #
> # creator

Can you add the GPL text and copyright here so the license is clear
and we know where it came from?

> owner=arekm@maven.pl
> 
> seq=`basename $0`
> echo "QA output created by $seq"
> 
> here=`pwd`
> tmp=/tmp/$$
> status=1	# failure is the default!
> 
> # get standard environment, filters and checks
> . ./common.rc
> . ./common.filter
> . ./common.quota
> 
> _cleanup()
> {
> 	cd /
> 	umount $SCRATCH_MNT 2>/dev/null
> 	rm -f $tmp.*
> }
> #trap "_cleanup; exit \$status" 0 1 2 3 15
> 
> 
> # real QA test starts here
> _supported_fs xfs
> _supported_os Linux IRIX

I'm not sure any will ever validate this on Irix, so maybe that
should be left out?

> 
> _require_xfs_quota
> 
> dir=$SCRATCH_MNT/project
> 
> _require_scratch
> _scratch_mkfs_xfs >/dev/null 2>&1
> 
> #if pquota's already in mount options then we dont need to enable
> 
> # we can't run with group quotas
> if ( `echo $MOUNT_OPTIONS | grep -q gquota` || `echo $MOUNT_OPTIONS | grep -q grpquota` )

if ( `echo $MOUNT_OPTIONS | egrep -q g??quota` )

> then
>     _notrun "Can't run with group quotas enabled"
> fi
> EXTRA_MOUNT_OPTIONS="-o pquota"
> 
> if ! _scratch_mount "$EXTRA_MOUNT_OPTIONS" >$tmp.out 2>&1
> then
>     cat $tmp.out
>     echo "!!! mount failed"
>     exit

	_fail "!!! failed to mount

> fi
> 
> src/feature -p $SCRATCH_DEV
> [ $? -ne 0 ] && _notrun "Installed kernel does not support project quotas"

This test should probably be first.

> mkdir $dir
> touch $dir/below16bit

status=0

> # below 16bit value
> xfs_quota -x -c "project -s -p $dir/below16bit 3422" $SCRATCH_DEV
> projid=$($XFS_IO_PROG -r -c "lsproj" $dir/below16bit)
> if [ "projid = 3422" != "$projid" ]; then
> 	echo "FAIL: returned projid value ($projid) doesn't match set one (3422)"

	status=1

> fi
> 
> # over 16bit value
> touch $dir/over16bit
> xfs_quota -x -c "project -s -p $dir/over16bit 108545" $SCRATCH_DEV
> projid=$($XFS_IO_PROG -r -c "lsproj" $dir)
> if [ "projid = 108545" != "$projid" ]; then
> 	echo "FAIL: returned projid value ($projid) doesn't match set one (108545)"

	status=1

> fi
> 
> # over 32bit value, should fail
> touch $dir/over32bit
> if ! xfs_quota -x -c "project -s -p $dir/over32bit 5344967296" $SCRATCH_DEV; then
> 	echo "FAIL: setting over 32bit projid succeeded while it should fail"

	status=1

> fi
> 
> # success, all done
> status=0

Remove these two lines.

> exit


Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH] xfstests: Quota project id setting overflow

[Arkadiusz Miśkiewicz]

From: Arkadiusz Miskiewicz <arekm@maven.pl>

Test 3 quota project setting id conditions:
- set 16bit project quota id -> should succeed
- set 32bit project quota id -> should succeed (with projid32bit
  patch applied; fail otherwise)
- over 32bit project quota id -> should always fail

Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl>
---
 243     |  121 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 243.out |    2 +
 group   |    1 +
 3 files changed, 124 insertions(+), 0 deletions(-)
 create mode 100644 243
 create mode 100644 243.out

diff --git a/243 b/243
new file mode 100644
index 0000000..34fa969
--- /dev/null
+++ b/243
@@ -0,0 +1,121 @@
+#! /bin/bash
+# FS QA Test No. 243
+#
+# test to verify that proper project quota id is correctly set
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2010 Arkadiusz Miśkiewicz.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#
+#-----------------------------------------------------------------------
+#
+# creator
+owner=arekm@maven.pl
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.quota
+
+_cleanup()
+{
+	cd /
+	umount $SCRATCH_MNT 2>/dev/null
+	rm -f $tmp.*
+}
+
+# real QA test starts here
+_supported_fs xfs
+
+_require_xfs_quota
+
+dir=$SCRATCH_MNT/project
+
+_require_scratch
+_scratch_mkfs_xfs >/dev/null 2>&1
+
+#if pquota's already in mount options then we dont need to enable
+# we can't run with group quotas
+if ( echo $MOUNT_OPTIONS | egrep -q g??quota )
+then
+    _notrun "Can't run with group quotas enabled"
+fi
+EXTRA_MOUNT_OPTIONS="-o pquota"
+
+if ! _scratch_mount "$EXTRA_MOUNT_OPTIONS" >$tmp.out 2>&1
+then
+    cat $tmp.out
+    _fail "!!! failed to mount"
+fi
+
+src/feature -p $SCRATCH_DEV
+[ $? -ne 0 ] && _notrun "Installed kernel does not support project quotas"
+
+status=0
+
+echo "Silence is golden"
+
+mkdir $dir
+touch $dir/below16bit
+# below 16bit value
+xfs_quota -x -c "project -s -p $dir/below16bit 3422" $SCRATCH_DEV >> $seq.full
+projid=$($XFS_IO_PROG -r -c "lsproj" $dir/below16bit)
+if [ "projid = 3422" != "$projid" ]; then
+	echo "FAIL: returned projid value ($projid) doesn't match set one (projid = 3422)"
+	status=1
+fi
+
+# 0x20 - projid32bit features2 bit
+features2_pre=$(xfs_db -x -r -c 'sb' -c 'print features2' $SCRATCH_DEV | awk ' { print $3 } ')
+projid32bit_pre=$((features2_pre & 0x20))
+# over 16bit value
+touch $dir/over16bit
+if (xfs_quota -x -c "project -s -p $dir/over16bit 108545" $SCRATCH_DEV >> $seq.full); then
+	# success? verify if projid matches
+	projid=$($XFS_IO_PROG -r -c "lsproj" $dir/over16bit)
+	if [ "projid = 108545" != "$projid" ]; then
+		echo "FAIL: returned projid value ($projid) doesn't match set one (projid = 108545)"
+		status=1
+	else
+		# if it really succeeded then projid32bit feature bit is supposed to be set automaticly
+		features2_post=$(xfs_db -x -r -c 'sb' -c 'print features2' $SCRATCH_DEV | awk ' { print $3 } ')
+		projid32bit_post=$((features2_post & 0x20))
+		if [ "$projid32bit_post" -eq 0 ]; then
+			echo "FAIL: setting 32bit projid succeeded but projid32bit features2 bit wasn't automaticly set"
+			status=1
+		fi
+	fi
+else
+	# didn't succeed but it should if projid32bit feature bit was already set
+	if [ "$projid32bit_pre" -gt 0 ]; then
+		echo "FAIL: setting 32bit projid failed but it should succeeded"
+		status=1
+	fi
+fi
+
+# over 32bit value, should fail
+touch $dir/over32bit
+if xfs_quota -x -c "project -s -p $dir/over32bit 5344967296" $SCRATCH_DEV >> $seq.full; then
+	echo "FAIL: setting over 32bit projid succeeded while it should fail"
+	status=1
+fi
diff --git a/243.out b/243.out
new file mode 100644
index 0000000..d075942
--- /dev/null
+++ b/243.out
@@ -0,0 +1,2 @@
+QA output created by 243
+Silence is golden
diff --git a/group b/group
index ff16bb3..e7ba59b 100644
--- a/group
+++ b/group
@@ -356,3 +356,4 @@ deprecated
 240 auto aio quick rw
 241 auto
 242 auto quick prealloc
+243 auto quota quick
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH] Validate string -> number conversion.

[Arkadiusz Miśkiewicz]

Make sure that numbers passed as string will fit into proper
types when doing string->uid_t/gid_t/prid_t conversion.

Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl>
---

Fixed version.

 libxcmd/input.c |   33 +++++++++++++++++++++------------
 quota/project.c |    2 +-
 2 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/libxcmd/input.c b/libxcmd/input.c
index 1bc0745..d0c4e0d 100644
--- a/libxcmd/input.c
+++ b/libxcmd/input.c
@@ -336,16 +336,19 @@ prid_from_string(
 	char		*project)
 {
 	fs_project_t	*prj;
-	prid_t		prid;
+	unsigned long int	prid_long;
 	char		*sp;
 
 	/*
 	 * Allow either a full numeric or a valid projectname, even
 	 * if it starts with a digit.
 	 */
-	prid = (prid_t)strtoul(project, &sp, 10);
-	if (*project != '\0' && *sp == '\0')
-		return prid;
+	prid_long = strtoul(project, &sp, 10);
+	if (*project != '\0' && *sp == '\0') {
+		if ((prid_long == ULONG_MAX && errno == ERANGE) || (prid_long > (prid_t)-1))
+			return -1;
+		return (prid_t)prid_long;
+	}
 	prj = getprnam(project);
 	if (prj)
 		return prj->pr_prid;
@@ -357,12 +360,15 @@ uid_from_string(
 	char		*user)
 {
 	struct passwd	*pwd;
-	uid_t		uid;
+	unsigned long int	uid_long;
 	char		*sp;
 
-	uid = (uid_t)strtoul(user, &sp, 10);
-	if (sp != user)
-		return uid;
+	uid_long = strtoul(user, &sp, 10);
+	if (sp != user) {
+		if ((uid_long == ULONG_MAX && errno == ERANGE) || (uid_long > (uid_t)-1))
+			return -1;
+		return (uid_t)uid_long;
+	}
 	pwd = getpwnam(user);
 	if (pwd)
 		return pwd->pw_uid;
@@ -374,12 +380,15 @@ gid_from_string(
 	char		*group)
 {
 	struct group	*grp;
-	gid_t		gid;
+	unsigned long int	gid_long;
 	char		*sp;
 
-	gid = (gid_t)strtoul(group, &sp, 10);
-	if (sp != group)
-		return gid;
+	gid_long = strtoul(group, &sp, 10);
+	if (sp != group) {
+		if ((gid_long == ULONG_MAX && errno == ERANGE) || (gid_long > (gid_t)-1))
+			return -1;
+		return (gid_t)gid_long;
+	}
 	grp = getgrnam(group);
 	if (grp)
 		return grp->gr_gid;
diff --git a/quota/project.c b/quota/project.c
index 1aacddd..e9baadd 100644
--- a/quota/project.c
+++ b/quota/project.c
@@ -331,7 +331,7 @@ project_f(
 		prid = prid_from_string(argv[optind]);
 		if (prid == -1) {
 			exitcode = 1;
-			fprintf(stderr, _("%s - no such project in %s\n"),
+			fprintf(stderr, _("%s - no such project in %s or invalid project number\n"),
 				argv[optind], projects_file);
 		} else
 	                project(argv[optind], type);
-- 
1.7.2.2

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] Validate string -> number conversion.

[Dave Chinner]

On Thu, Aug 26, 2010 at 09:30:52AM +0200, Arkadiusz Miśkiewicz wrote:
> Make sure that numbers passed as string will fit into proper
> types when doing string->uid_t/gid_t/prid_t conversion.
> 
> Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl>

Looks good, but just a couple of minor style things:

> ---
> 
> Fixed version.
> 
>  libxcmd/input.c |   33 +++++++++++++++++++++------------
>  quota/project.c |    2 +-
>  2 files changed, 22 insertions(+), 13 deletions(-)
> 
> diff --git a/libxcmd/input.c b/libxcmd/input.c
> index 1bc0745..d0c4e0d 100644
> --- a/libxcmd/input.c
> +++ b/libxcmd/input.c
> @@ -336,16 +336,19 @@ prid_from_string(
>  	char		*project)
>  {
>  	fs_project_t	*prj;
> -	prid_t		prid;
> +	unsigned long int	prid_long;
>  	char		*sp;

unsigned long is all that is necessary here - that will also keep
the same indent as well.
>  
>  	/*
>  	 * Allow either a full numeric or a valid projectname, even
>  	 * if it starts with a digit.
>  	 */
> -	prid = (prid_t)strtoul(project, &sp, 10);
> -	if (*project != '\0' && *sp == '\0')
> -		return prid;
> +	prid_long = strtoul(project, &sp, 10);
> +	if (*project != '\0' && *sp == '\0') {
> +		if ((prid_long == ULONG_MAX && errno == ERANGE) || (prid_long > (prid_t)-1))

Can you split this to keep within 80 chars?

Same for the uid and gid checks as well.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH] Validate string -> number conversion. [version 3]

[Arkadiusz Miśkiewicz]

Make sure that numbers passed as string will fit into proper
types when doing string->uid_t/gid_t/prid_t conversion.

Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl>
---
 libxcmd/input.c |   36 ++++++++++++++++++++++++------------
 quota/project.c |    2 +-
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/libxcmd/input.c b/libxcmd/input.c
index 1bc0745..d7f29c1 100644
--- a/libxcmd/input.c
+++ b/libxcmd/input.c
@@ -336,16 +336,20 @@ prid_from_string(
 	char		*project)
 {
 	fs_project_t	*prj;
-	prid_t		prid;
+	unsigned long	prid_long;
 	char		*sp;
 
 	/*
 	 * Allow either a full numeric or a valid projectname, even
 	 * if it starts with a digit.
 	 */
-	prid = (prid_t)strtoul(project, &sp, 10);
-	if (*project != '\0' && *sp == '\0')
-		return prid;
+	prid_long = strtoul(project, &sp, 10);
+	if (*project != '\0' && *sp == '\0') {
+		if ((prid_long == ULONG_MAX && errno == ERANGE)
+				|| (prid_long > (prid_t)-1))
+			return -1;
+		return (prid_t)prid_long;
+	}
 	prj = getprnam(project);
 	if (prj)
 		return prj->pr_prid;
@@ -357,12 +361,16 @@ uid_from_string(
 	char		*user)
 {
 	struct passwd	*pwd;
-	uid_t		uid;
+	unsigned long	uid_long;
 	char		*sp;
 
-	uid = (uid_t)strtoul(user, &sp, 10);
-	if (sp != user)
-		return uid;
+	uid_long = strtoul(user, &sp, 10);
+	if (sp != user) {
+		if ((uid_long == ULONG_MAX && errno == ERANGE)
+				|| (uid_long > (uid_t)-1))
+			return -1;
+		return (uid_t)uid_long;
+	}
 	pwd = getpwnam(user);
 	if (pwd)
 		return pwd->pw_uid;
@@ -374,12 +382,16 @@ gid_from_string(
 	char		*group)
 {
 	struct group	*grp;
-	gid_t		gid;
+	unsigned long	gid_long;
 	char		*sp;
 
-	gid = (gid_t)strtoul(group, &sp, 10);
-	if (sp != group)
-		return gid;
+	gid_long = strtoul(group, &sp, 10);
+	if (sp != group) {
+		if ((gid_long == ULONG_MAX && errno == ERANGE)
+				|| (gid_long > (gid_t)-1))
+			return -1;
+		return (gid_t)gid_long;
+	}
 	grp = getgrnam(group);
 	if (grp)
 		return grp->gr_gid;
diff --git a/quota/project.c b/quota/project.c
index 1aacddd..e9baadd 100644
--- a/quota/project.c
+++ b/quota/project.c
@@ -331,7 +331,7 @@ project_f(
 		prid = prid_from_string(argv[optind]);
 		if (prid == -1) {
 			exitcode = 1;
-			fprintf(stderr, _("%s - no such project in %s\n"),
+			fprintf(stderr, _("%s - no such project in %s or invalid project number\n"),
 				argv[optind], projects_file);
 		} else
 	                project(argv[optind], type);
-- 
1.7.2.2

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] Validate string -> number conversion. [version 3]

[Alex Elder]

On Fri, 2010-08-27 at 22:54 +0200, Arkadiusz Miśkiewicz wrote:
> Make sure that numbers passed as string will fit into proper
> types when doing string->uid_t/gid_t/prid_t conversion.

Looks good to me.  I can commit this for you next week if
nobody else has any issue with it.

Reviewed-by: Alex Elder <aelder@sgi.com>

> Signed-off-by: Arkadiusz Miśkiewicz <arekm@maven.pl>
> ---
>  libxcmd/input.c |   36 ++++++++++++++++++++++++------------
>  quota/project.c |    2 +-
>  2 files changed, 25 insertions(+), 13 deletions(-)
. . .

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] Validate string -> number conversion. [version 3]

[Christoph Hellwig]

On Fri, Aug 27, 2010 at 10:54:36PM +0200, Arkadiusz Mi??kiewicz wrote:
> Make sure that numbers passed as string will fit into proper
> types when doing string->uid_t/gid_t/prid_t conversion.

Looks good,


Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs