| To: | xfs@xxxxxxxxxxx |
|---|---|
| Subject: | [PATCH 06/17] mkfs: validate logarithmic parameters sanely |
| From: | Jan ÅulÃk <jtulak@xxxxxxxxxx> |
| Date: | Fri, 19 Jun 2015 13:01:55 +0200 |
| Cc: | david@xxxxxxxxxxxxx, Dave Chinner <dchinner@xxxxxxxxxx> |
| Delivered-to: | xfs@xxxxxxxxxxx |
| In-reply-to: | <1434711726-13092-1-git-send-email-jtulak@xxxxxxxxxx> |
| References: | <1434711726-13092-1-git-send-email-jtulak@xxxxxxxxxx> |
From: Dave Chinner <dchinner@xxxxxxxxxx>
Testing logarithmic paramters like "-n log=<num>" shows that we do a
terrible job of validating such input. e.g.:
# mkfs.xfs -f -n log=456858480 /dev/vda
.....
naming =version 2 bsize=65536 ascii-ci=0 ftype=0
....
Yeah, I just asked for a block size of 2^456858480, and it didn't
get rejected. Great, isn't it?
So, factor out the parsing of logarithmic parameters, and pass in
the maximum valid value that they can take. These maximum values
might not be completely accurate (e.g. block/sector sizes will
affect the eventual valid maximum) but we can get rid of all the
overflows and stupidities before we get to fine-grained validity
checking later in mkfs once things like block and sector sizes have
been finalised.
Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
Signed-off-by: Jan ÅulÃk <jtulak@xxxxxxxxxx>
---
mkfs/xfs_mkfs.c | 79 +++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 51 insertions(+), 28 deletions(-)
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 6b9e991..6f0aa55 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -1044,6 +1044,27 @@ getbool(
return c ? true : false;
}
+static int
+getnum_checked(
+ const char *str,
+ long long min_val,
+ long long max_val,
+ const char *illegal_str,
+ char reqval_char,
+ char *reqval_opts[],
+ int reqval_optind)
+{
+ long long c;
+
+ if (!str || *str == '\0')
+ reqval(reqval_char, reqval_opts, reqval_optind);
+
+ c = getnum(str, 0, 0, false);
+ if (c < min_val || c > max_val)
+ illegal(str, illegal_str);
+ return c;
+}
+
int
main(
int argc,
@@ -1200,16 +1221,16 @@ main(
switch (getsubopt(&p, (constpp)bopts, &value)) {
case B_LOG:
- if (!value || *value == '\0')
- reqval('b', bopts, B_LOG);
if (blflag)
respec('b', bopts, B_LOG);
if (bsflag)
conflict('b', bopts, B_SIZE,
B_LOG);
- blocklog = getnum(value, 0, 0, false);
- if (blocklog <= 0)
- illegal(value, "b log");
+ blocklog = getnum_checked(value,
+ XFS_MIN_BLOCKSIZE_LOG,
+ XFS_MAX_BLOCKSIZE_LOG,
+ "b log", 'b', bopts,
+ B_LOG);
blocksize = 1 << blocklog;
blflag = 1;
break;
@@ -1346,16 +1367,16 @@ main(
nodsflag = 1;
break;
case D_SECTLOG:
- if (!value || *value == '\0')
- reqval('d', dopts, D_SECTLOG);
if (slflag)
respec('d', dopts, D_SECTLOG);
if (ssflag)
conflict('d', dopts, D_SECTSIZE,
D_SECTLOG);
- sectorlog = getnum(value, 0, 0, false);
- if (sectorlog <= 0)
- illegal(value, "d sectlog");
+ sectorlog = getnum_checked(value,
+ XFS_MIN_SECTORSIZE_LOG,
+ XFS_MAX_SECTORSIZE_LOG,
+ "d sectlog", 'd', dopts,
+ D_SECTLOG);
sectorsize = 1 << sectorlog;
slflag = 1;
break;
@@ -1420,9 +1441,11 @@ main(
if (isflag)
conflict('i', iopts, I_SIZE,
I_LOG);
- inodelog = getnum(value, 0, 0, false);
- if (inodelog <= 0)
- illegal(value, "i log");
+ inodelog = getnum_checked(value,
+ XFS_DINODE_MIN_LOG,
+ XFS_DINODE_MAX_LOG,
+ "i log", 'i', iopts,
+ I_LOG);
isize = 1 << inodelog;
ilflag = 1;
break;
@@ -1591,16 +1614,16 @@ main(
lsflag = 1;
break;
case L_SECTLOG:
- if (!value || *value == '\0')
- reqval('l', lopts, L_SECTLOG);
if (lslflag)
respec('l', lopts, L_SECTLOG);
if (lssflag)
conflict('l', lopts, L_SECTSIZE,
L_SECTLOG);
- lsectorlog = getnum(value, 0, 0, false);
- if (lsectorlog <= 0)
- illegal(value, "l sectlog");
+ lsectorlog = getnum_checked(value,
+ XFS_MIN_SECTORSIZE_LOG,
+ XFS_MAX_SECTORSIZE_LOG,
+ "l sectlog", 'l', lopts,
+ L_SECTLOG);
lsectorsize = 1 << lsectorlog;
lslflag = 1;
break;
@@ -1673,16 +1696,16 @@ _("cannot specify both -m crc=1 and -n ftype\n"));
switch (getsubopt(&p, (constpp)nopts, &value)) {
case N_LOG:
- if (!value || *value == '\0')
- reqval('n', nopts, N_LOG);
if (nlflag)
respec('n', nopts, N_LOG);
if (nsflag)
conflict('n', nopts, N_SIZE,
N_LOG);
- dirblocklog = getnum(value, 0, 0,
false);
- if (dirblocklog <= 0)
- illegal(value, "n log");
+ dirblocklog = getnum_checked(value,
+ XFS_MIN_REC_DIRSIZE,
+ XFS_MAX_BLOCKSIZE_LOG,
+ "n log", 'n', nopts,
+ N_LOG);
dirblocksize = 1 << dirblocklog;
nlflag = 1;
break;
@@ -1801,16 +1824,16 @@ _("cannot specify both -m crc=1 and -n ftype\n"));
switch (getsubopt(&p, (constpp)sopts, &value)) {
case S_LOG:
case S_SECTLOG:
- if (!value || *value == '\0')
- reqval('s', sopts, S_SECTLOG);
if (slflag || lslflag)
respec('s', sopts, S_SECTLOG);
if (ssflag || lssflag)
conflict('s', sopts, S_SECTSIZE,
S_SECTLOG);
- sectorlog = getnum(value, 0, 0, false);
- if (sectorlog <= 0)
- illegal(value, "s sectlog");
+ sectorlog = getnum_checked(value,
+ XFS_MIN_SECTORSIZE_LOG,
+ XFS_MAX_SECTORSIZE_LOG,
+ "s sectlog", 's', sopts,
+ S_SECTLOG);
lsectorlog = sectorlog;
sectorsize = 1 << sectorlog;
lsectorsize = sectorsize;
--
2.1.0
|
| <Prev in Thread] | Current Thread | [Next in Thread> |
|---|---|---|
| ||
| Previous by Date: | [PATCH 10/17] mkfs: add respecification detection to generic parsing, Jan ÅulÃk |
|---|---|
| Next by Date: | [PATCH 11/17] mkfs: table based parsing for converted parameters, Jan ÅulÃk |
| Previous by Thread: | Re: [PATCH 10/17] mkfs: add respecification detection to generic parsing, Brian Foster |
| Next by Thread: | Re: [PATCH 06/17] mkfs: validate logarithmic parameters sanely, Brian Foster |
| Indexes: | [Date] [Thread] [Top] [All Lists] |