Re: [PATCH] tpm2: fix off-by-one comparison and out-of-bounds read error
On Thu, Mar 23, 2017 at 07:42:09PM +0300, Dan Carpenter wrote: > On Thu, Mar 23, 2017 at 05:53:58PM +0200, Jarkko Sakkinen wrote: > > On Wed, Mar 22, 2017 at 04:12:49PM +0300, Dan Carpenter wrote: > > > On Wed, Mar 22, 2017 at 11:45:37AM +, Colin Ian King wrote: > > > > On 22/03/17 11:42, Jarkko Sakkinen wrote: > > > > > On Mon, Mar 20, 2017 at 02:23:36PM +, Colin King wrote: > > > > >> From: Colin Ian King > > > > >> > > > > >> The comparison of an out of range index into space->context_tbl is > > > > >> off-by-one and should be using >= rather than > in the comparison. > > > > >> > > > > >> Detected by CoverityScan, CID#1419694 ("Out-of-bounds read") > > > > >> > > > > >> Fixes: 849246e7ce9ce ("tpm2: add session handle context saving and > > > > >> restoring to the space code") > > > > >> Signed-off-by: Colin Ian King > > > > >> --- > > > > >> drivers/char/tpm/tpm2-space.c | 2 +- > > > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > > > >> > > > > >> diff --git a/drivers/char/tpm/tpm2-space.c > > > > >> b/drivers/char/tpm/tpm2-space.c > > > > >> index d36d81e07076..009934269514 100644 > > > > >> --- a/drivers/char/tpm/tpm2-space.c > > > > >> +++ b/drivers/char/tpm/tpm2-space.c > > > > >> @@ -229,7 +229,7 @@ static bool tpm2_map_to_phandle(struct tpm_space > > > > >> *space, void *handle) > > > > >> int i; > > > > >> > > > > >> i = 0xFF - (vhandle & 0xFF); > > > > >> -if (i > ARRAY_SIZE(space->context_tbl) || > > > > >> !space->context_tbl[i]) > > > > >> +if (i >= ARRAY_SIZE(space->context_tbl) || > > > > >> !space->context_tbl[i]) > > > > >> return false; > > > > >> > > > > >> phandle = space->context_tbl[i]; > > > > >> -- > > > > >> 2.11.0 > > > > > > > > > > Thanks. If you don't mind, I would squash this to that patch? > > > > > > > > > > /Jarkko > > > > > > > > > Sure squash it, and maybe add a Reported-by: Colin Ian King > > > > if that's OK with you. > > > > > > Reported-by isn't really correct though... We should have a Fixes-from: > > > tag for squashed fixes. > > > > > > regards, > > > dan carpenter > > > > Hmm... Maybe so depending on how you interpret Reported-by but > > Fixes-from is not something that is used at the moment, is it? > > No, but we should create it. > > regards, > dan carpenter Fully agreed. Using reported-by in the commit in question implies as if the whole commit was proposed by Colin. /Jarkko
Re: [PATCH] tpm2: fix off-by-one comparison and out-of-bounds read error
On Thu, Mar 23, 2017 at 05:53:58PM +0200, Jarkko Sakkinen wrote: > On Wed, Mar 22, 2017 at 04:12:49PM +0300, Dan Carpenter wrote: > > On Wed, Mar 22, 2017 at 11:45:37AM +, Colin Ian King wrote: > > > On 22/03/17 11:42, Jarkko Sakkinen wrote: > > > > On Mon, Mar 20, 2017 at 02:23:36PM +, Colin King wrote: > > > >> From: Colin Ian King > > > >> > > > >> The comparison of an out of range index into space->context_tbl is > > > >> off-by-one and should be using >= rather than > in the comparison. > > > >> > > > >> Detected by CoverityScan, CID#1419694 ("Out-of-bounds read") > > > >> > > > >> Fixes: 849246e7ce9ce ("tpm2: add session handle context saving and > > > >> restoring to the space code") > > > >> Signed-off-by: Colin Ian King > > > >> --- > > > >> drivers/char/tpm/tpm2-space.c | 2 +- > > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > > >> > > > >> diff --git a/drivers/char/tpm/tpm2-space.c > > > >> b/drivers/char/tpm/tpm2-space.c > > > >> index d36d81e07076..009934269514 100644 > > > >> --- a/drivers/char/tpm/tpm2-space.c > > > >> +++ b/drivers/char/tpm/tpm2-space.c > > > >> @@ -229,7 +229,7 @@ static bool tpm2_map_to_phandle(struct tpm_space > > > >> *space, void *handle) > > > >>int i; > > > >> > > > >>i = 0xFF - (vhandle & 0xFF); > > > >> - if (i > ARRAY_SIZE(space->context_tbl) || > > > >> !space->context_tbl[i]) > > > >> + if (i >= ARRAY_SIZE(space->context_tbl) || > > > >> !space->context_tbl[i]) > > > >>return false; > > > >> > > > >>phandle = space->context_tbl[i]; > > > >> -- > > > >> 2.11.0 > > > > > > > > Thanks. If you don't mind, I would squash this to that patch? > > > > > > > > /Jarkko > > > > > > > Sure squash it, and maybe add a Reported-by: Colin Ian King > > > if that's OK with you. > > > > Reported-by isn't really correct though... We should have a Fixes-from: > > tag for squashed fixes. > > > > regards, > > dan carpenter > > Hmm... Maybe so depending on how you interpret Reported-by but > Fixes-from is not something that is used at the moment, is it? No, but we should create it. regards, dan carpenter
Re: [PATCH] tpm2: fix off-by-one comparison and out-of-bounds read error
On Thu, Mar 23, 2017 at 05:53:58PM +0200, Jarkko Sakkinen wrote: > On Wed, Mar 22, 2017 at 04:12:49PM +0300, Dan Carpenter wrote: > > On Wed, Mar 22, 2017 at 11:45:37AM +, Colin Ian King wrote: > > > On 22/03/17 11:42, Jarkko Sakkinen wrote: > > > > On Mon, Mar 20, 2017 at 02:23:36PM +, Colin King wrote: > > > >> From: Colin Ian King > > > >> > > > >> The comparison of an out of range index into space->context_tbl is > > > >> off-by-one and should be using >= rather than > in the comparison. > > > >> > > > >> Detected by CoverityScan, CID#1419694 ("Out-of-bounds read") > > > >> > > > >> Fixes: 849246e7ce9ce ("tpm2: add session handle context saving and > > > >> restoring to the space code") > > > >> Signed-off-by: Colin Ian King > > > >> --- > > > >> drivers/char/tpm/tpm2-space.c | 2 +- > > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > > >> > > > >> diff --git a/drivers/char/tpm/tpm2-space.c > > > >> b/drivers/char/tpm/tpm2-space.c > > > >> index d36d81e07076..009934269514 100644 > > > >> --- a/drivers/char/tpm/tpm2-space.c > > > >> +++ b/drivers/char/tpm/tpm2-space.c > > > >> @@ -229,7 +229,7 @@ static bool tpm2_map_to_phandle(struct tpm_space > > > >> *space, void *handle) > > > >>int i; > > > >> > > > >>i = 0xFF - (vhandle & 0xFF); > > > >> - if (i > ARRAY_SIZE(space->context_tbl) || > > > >> !space->context_tbl[i]) > > > >> + if (i >= ARRAY_SIZE(space->context_tbl) || > > > >> !space->context_tbl[i]) > > > >>return false; > > > >> > > > >>phandle = space->context_tbl[i]; > > > >> -- > > > >> 2.11.0 > > > > > > > > Thanks. If you don't mind, I would squash this to that patch? > > > > > > > > /Jarkko > > > > > > > Sure squash it, and maybe add a Reported-by: Colin Ian King > > > if that's OK with you. > > > > Reported-by isn't really correct though... We should have a Fixes-from: > > tag for squashed fixes. > > > > regards, > > dan carpenter > > Hmm... Maybe so depending on how you interpret Reported-by but > Fixes-from is not something that is used at the moment, is it? > > /Jarkko When I started squashing the commit I realized what you meant so I'm adding this to the commit: Fixes-from: Colin Ian King /Jarkko
Re: [PATCH] tpm2: fix off-by-one comparison and out-of-bounds read error
On Wed, Mar 22, 2017 at 04:12:49PM +0300, Dan Carpenter wrote: > On Wed, Mar 22, 2017 at 11:45:37AM +, Colin Ian King wrote: > > On 22/03/17 11:42, Jarkko Sakkinen wrote: > > > On Mon, Mar 20, 2017 at 02:23:36PM +, Colin King wrote: > > >> From: Colin Ian King > > >> > > >> The comparison of an out of range index into space->context_tbl is > > >> off-by-one and should be using >= rather than > in the comparison. > > >> > > >> Detected by CoverityScan, CID#1419694 ("Out-of-bounds read") > > >> > > >> Fixes: 849246e7ce9ce ("tpm2: add session handle context saving and > > >> restoring to the space code") > > >> Signed-off-by: Colin Ian King > > >> --- > > >> drivers/char/tpm/tpm2-space.c | 2 +- > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > >> > > >> diff --git a/drivers/char/tpm/tpm2-space.c > > >> b/drivers/char/tpm/tpm2-space.c > > >> index d36d81e07076..009934269514 100644 > > >> --- a/drivers/char/tpm/tpm2-space.c > > >> +++ b/drivers/char/tpm/tpm2-space.c > > >> @@ -229,7 +229,7 @@ static bool tpm2_map_to_phandle(struct tpm_space > > >> *space, void *handle) > > >> int i; > > >> > > >> i = 0xFF - (vhandle & 0xFF); > > >> -if (i > ARRAY_SIZE(space->context_tbl) || > > >> !space->context_tbl[i]) > > >> +if (i >= ARRAY_SIZE(space->context_tbl) || > > >> !space->context_tbl[i]) > > >> return false; > > >> > > >> phandle = space->context_tbl[i]; > > >> -- > > >> 2.11.0 > > > > > > Thanks. If you don't mind, I would squash this to that patch? > > > > > > /Jarkko > > > > > Sure squash it, and maybe add a Reported-by: Colin Ian King > > if that's OK with you. > > Reported-by isn't really correct though... We should have a Fixes-from: > tag for squashed fixes. > > regards, > dan carpenter Hmm... Maybe so depending on how you interpret Reported-by but Fixes-from is not something that is used at the moment, is it? /Jarkko
Re: [PATCH] tpm2: fix off-by-one comparison and out-of-bounds read error
On Wed, Mar 22, 2017 at 11:45:37AM +, Colin Ian King wrote: > On 22/03/17 11:42, Jarkko Sakkinen wrote: > > On Mon, Mar 20, 2017 at 02:23:36PM +, Colin King wrote: > >> From: Colin Ian King > >> > >> The comparison of an out of range index into space->context_tbl is > >> off-by-one and should be using >= rather than > in the comparison. > >> > >> Detected by CoverityScan, CID#1419694 ("Out-of-bounds read") > >> > >> Fixes: 849246e7ce9ce ("tpm2: add session handle context saving and > >> restoring to the space code") > >> Signed-off-by: Colin Ian King > >> --- > >> drivers/char/tpm/tpm2-space.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c > >> index d36d81e07076..009934269514 100644 > >> --- a/drivers/char/tpm/tpm2-space.c > >> +++ b/drivers/char/tpm/tpm2-space.c > >> @@ -229,7 +229,7 @@ static bool tpm2_map_to_phandle(struct tpm_space > >> *space, void *handle) > >>int i; > >> > >>i = 0xFF - (vhandle & 0xFF); > >> - if (i > ARRAY_SIZE(space->context_tbl) || !space->context_tbl[i]) > >> + if (i >= ARRAY_SIZE(space->context_tbl) || !space->context_tbl[i]) > >>return false; > >> > >>phandle = space->context_tbl[i]; > >> -- > >> 2.11.0 > > > > Thanks. If you don't mind, I would squash this to that patch? > > > > /Jarkko > > > Sure squash it, and maybe add a Reported-by: Colin Ian King > if that's OK with you. > > Colin Thanks I'll do that! /Jarkko
Re: [PATCH] tpm2: fix off-by-one comparison and out-of-bounds read error
On Wed, Mar 22, 2017 at 11:45:37AM +, Colin Ian King wrote: > On 22/03/17 11:42, Jarkko Sakkinen wrote: > > On Mon, Mar 20, 2017 at 02:23:36PM +, Colin King wrote: > >> From: Colin Ian King > >> > >> The comparison of an out of range index into space->context_tbl is > >> off-by-one and should be using >= rather than > in the comparison. > >> > >> Detected by CoverityScan, CID#1419694 ("Out-of-bounds read") > >> > >> Fixes: 849246e7ce9ce ("tpm2: add session handle context saving and > >> restoring to the space code") > >> Signed-off-by: Colin Ian King > >> --- > >> drivers/char/tpm/tpm2-space.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c > >> index d36d81e07076..009934269514 100644 > >> --- a/drivers/char/tpm/tpm2-space.c > >> +++ b/drivers/char/tpm/tpm2-space.c > >> @@ -229,7 +229,7 @@ static bool tpm2_map_to_phandle(struct tpm_space > >> *space, void *handle) > >>int i; > >> > >>i = 0xFF - (vhandle & 0xFF); > >> - if (i > ARRAY_SIZE(space->context_tbl) || !space->context_tbl[i]) > >> + if (i >= ARRAY_SIZE(space->context_tbl) || !space->context_tbl[i]) > >>return false; > >> > >>phandle = space->context_tbl[i]; > >> -- > >> 2.11.0 > > > > Thanks. If you don't mind, I would squash this to that patch? > > > > /Jarkko > > > Sure squash it, and maybe add a Reported-by: Colin Ian King > if that's OK with you. Reported-by isn't really correct though... We should have a Fixes-from: tag for squashed fixes. regards, dan carpenter
Re: [PATCH] tpm2: fix off-by-one comparison and out-of-bounds read error
On Mon, Mar 20, 2017 at 02:23:36PM +, Colin King wrote: > From: Colin Ian King > > The comparison of an out of range index into space->context_tbl is > off-by-one and should be using >= rather than > in the comparison. > > Detected by CoverityScan, CID#1419694 ("Out-of-bounds read") > > Fixes: 849246e7ce9ce ("tpm2: add session handle context saving and restoring > to the space code") > Signed-off-by: Colin Ian King > --- > drivers/char/tpm/tpm2-space.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c > index d36d81e07076..009934269514 100644 > --- a/drivers/char/tpm/tpm2-space.c > +++ b/drivers/char/tpm/tpm2-space.c > @@ -229,7 +229,7 @@ static bool tpm2_map_to_phandle(struct tpm_space *space, > void *handle) > int i; > > i = 0xFF - (vhandle & 0xFF); > - if (i > ARRAY_SIZE(space->context_tbl) || !space->context_tbl[i]) > + if (i >= ARRAY_SIZE(space->context_tbl) || !space->context_tbl[i]) > return false; > > phandle = space->context_tbl[i]; > -- > 2.11.0 Thanks. If you don't mind, I would squash this to that patch? /Jarkko
Re: [PATCH] tpm2: fix off-by-one comparison and out-of-bounds read error
On 22/03/17 11:42, Jarkko Sakkinen wrote: > On Mon, Mar 20, 2017 at 02:23:36PM +, Colin King wrote: >> From: Colin Ian King >> >> The comparison of an out of range index into space->context_tbl is >> off-by-one and should be using >= rather than > in the comparison. >> >> Detected by CoverityScan, CID#1419694 ("Out-of-bounds read") >> >> Fixes: 849246e7ce9ce ("tpm2: add session handle context saving and restoring >> to the space code") >> Signed-off-by: Colin Ian King >> --- >> drivers/char/tpm/tpm2-space.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c >> index d36d81e07076..009934269514 100644 >> --- a/drivers/char/tpm/tpm2-space.c >> +++ b/drivers/char/tpm/tpm2-space.c >> @@ -229,7 +229,7 @@ static bool tpm2_map_to_phandle(struct tpm_space *space, >> void *handle) >> int i; >> >> i = 0xFF - (vhandle & 0xFF); >> -if (i > ARRAY_SIZE(space->context_tbl) || !space->context_tbl[i]) >> +if (i >= ARRAY_SIZE(space->context_tbl) || !space->context_tbl[i]) >> return false; >> >> phandle = space->context_tbl[i]; >> -- >> 2.11.0 > > Thanks. If you don't mind, I would squash this to that patch? > > /Jarkko > Sure squash it, and maybe add a Reported-by: Colin Ian King if that's OK with you. Colin
[PATCH] tpm2: fix off-by-one comparison and out-of-bounds read error
From: Colin Ian King The comparison of an out of range index into space->context_tbl is off-by-one and should be using >= rather than > in the comparison. Detected by CoverityScan, CID#1419694 ("Out-of-bounds read") Fixes: 849246e7ce9ce ("tpm2: add session handle context saving and restoring to the space code") Signed-off-by: Colin Ian King --- drivers/char/tpm/tpm2-space.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c index d36d81e07076..009934269514 100644 --- a/drivers/char/tpm/tpm2-space.c +++ b/drivers/char/tpm/tpm2-space.c @@ -229,7 +229,7 @@ static bool tpm2_map_to_phandle(struct tpm_space *space, void *handle) int i; i = 0xFF - (vhandle & 0xFF); - if (i > ARRAY_SIZE(space->context_tbl) || !space->context_tbl[i]) + if (i >= ARRAY_SIZE(space->context_tbl) || !space->context_tbl[i]) return false; phandle = space->context_tbl[i]; -- 2.11.0