linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tpm: Do not dereference NULL pointer if acpi_os_map_memory() fails.
@ 2012-08-07 20:50 Jesper Juhl
  2012-08-08 18:07 ` Kent Yoder
  0 siblings, 1 reply; 6+ messages in thread
From: Jesper Juhl @ 2012-08-07 20:50 UTC (permalink / raw)
  To: linux-kernel
  Cc: tpmdd-devel, Sirrix AG, Marcel Selhorst, Rajiv Andrade,
	Kent Yoder, Seiji Munetoh, Stefan Berger, Reiner Sailer,
	Kylene Hall

In drivers/char/tpm/tpm_bios.c::read_log() we call
acpi_os_map_memory(). That call may fail for a number of reasons
(invallid address, out of memory etc). If the call fails it returns
NULL and we just pass that to memcpy() unconditionally, which will go
bad when it tries to dereference the pointer.

Unfortunately we just get NULL back, so we can't really tell the user
exactely what went wrong, but we can at least avoid crashing and
return an error (-EIO seemed more generic and more suitable here than
-ENOMEM or something else, so I picked that).

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/char/tpm/tpm_bios.c | 5 +++++
 1 file changed, 5 insertions(+)

  Compile tested only.

diff --git a/drivers/char/tpm/tpm_bios.c b/drivers/char/tpm/tpm_bios.c
index 0636520..0c5c274 100644
--- a/drivers/char/tpm/tpm_bios.c
+++ b/drivers/char/tpm/tpm_bios.c
@@ -410,6 +410,11 @@ static int read_log(struct tpm_bios_log *log)
 	log->bios_event_log_end = log->bios_event_log + len;
 
 	virt = acpi_os_map_memory(start, len);
+	if (!virt) {
+		printk("%s: ERROR - Unable to map memory\n",
+			__func__);
+		return -EIO;
+	}
 
 	memcpy(log->bios_event_log, virt, len);
 
-- 
1.7.11.4


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] tpm: Do not dereference NULL pointer if acpi_os_map_memory() fails.
  2012-08-07 20:50 [PATCH] tpm: Do not dereference NULL pointer if acpi_os_map_memory() fails Jesper Juhl
@ 2012-08-08 18:07 ` Kent Yoder
  2012-08-15 20:15   ` Kent Yoder
  0 siblings, 1 reply; 6+ messages in thread
From: Kent Yoder @ 2012-08-08 18:07 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: linux-kernel, tpmdd-devel, Sirrix AG, Marcel Selhorst,
	Rajiv Andrade, Seiji Munetoh, Stefan Berger, Reiner Sailer,
	Kylene Hall

On Tue, Aug 07, 2012 at 10:50:56PM +0200, Jesper Juhl wrote:
> In drivers/char/tpm/tpm_bios.c::read_log() we call
> acpi_os_map_memory(). That call may fail for a number of reasons
> (invallid address, out of memory etc). If the call fails it returns
> NULL and we just pass that to memcpy() unconditionally, which will go
> bad when it tries to dereference the pointer.
> 
> Unfortunately we just get NULL back, so we can't really tell the user
> exactely what went wrong, but we can at least avoid crashing and
> return an error (-EIO seemed more generic and more suitable here than
> -ENOMEM or something else, so I picked that).

Thanks Jesper. I'd made some updates to tpm_bios.c recently but this
should still apply. I'll let you know if not.

Kent

> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
>  drivers/char/tpm/tpm_bios.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
>   Compile tested only.
> 
> diff --git a/drivers/char/tpm/tpm_bios.c b/drivers/char/tpm/tpm_bios.c
> index 0636520..0c5c274 100644
> --- a/drivers/char/tpm/tpm_bios.c
> +++ b/drivers/char/tpm/tpm_bios.c
> @@ -410,6 +410,11 @@ static int read_log(struct tpm_bios_log *log)
>  	log->bios_event_log_end = log->bios_event_log + len;
> 
>  	virt = acpi_os_map_memory(start, len);
> +	if (!virt) {
> +		printk("%s: ERROR - Unable to map memory\n",
> +			__func__);
> +		return -EIO;
> +	}
> 
>  	memcpy(log->bios_event_log, virt, len);
> 
> -- 
> 1.7.11.4
> 
> 
> -- 
> Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
> Don't top-post http://www.catb.org/jargon/html/T/top-post.html
> Plain text mails only, please.
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] tpm: Do not dereference NULL pointer if acpi_os_map_memory() fails.
  2012-08-08 18:07 ` Kent Yoder
@ 2012-08-15 20:15   ` Kent Yoder
  2012-08-15 22:02     ` Jesper Juhl
  0 siblings, 1 reply; 6+ messages in thread
From: Kent Yoder @ 2012-08-15 20:15 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: linux-kernel, tpmdd-devel, Sirrix AG, Marcel Selhorst,
	Rajiv Andrade, Seiji Munetoh, Stefan Berger, Reiner Sailer,
	Kylene Hall

Hi Jesper,

> > Unfortunately we just get NULL back, so we can't really tell the user
> > exactely what went wrong, but we can at least avoid crashing and
> > return an error (-EIO seemed more generic and more suitable here than
> > -ENOMEM or something else, so I picked that).
> 
> Thanks Jesper. I'd made some updates to tpm_bios.c recently but this
> should still apply. I'll let you know if not.

  Of course I'm wrong here, this code moved over into tpm_acpi.c. If you
can resubmit on top of my staging tree, I will apply it there.

git://github.com/shpedoikal/linux.git v3.6-rc1-tpmdd-staging

> > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > ---
> >  drivers/char/tpm/tpm_bios.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> >   Compile tested only.
> > 
> > diff --git a/drivers/char/tpm/tpm_bios.c b/drivers/char/tpm/tpm_bios.c
> > index 0636520..0c5c274 100644
> > --- a/drivers/char/tpm/tpm_bios.c
> > +++ b/drivers/char/tpm/tpm_bios.c
> > @@ -410,6 +410,11 @@ static int read_log(struct tpm_bios_log *log)
> >  	log->bios_event_log_end = log->bios_event_log + len;
> > 
> >  	virt = acpi_os_map_memory(start, len);
> > +	if (!virt) {

  Also please add kfree(log->bios_event_log); in this error path.

Thanks,
Kent

> > +		printk("%s: ERROR - Unable to map memory\n",
> > +			__func__);
> > +		return -EIO;
> > +	}
> > 
> >  	memcpy(log->bios_event_log, virt, len);
> > 
> > -- 
> > 1.7.11.4
> > 
> > 
> > -- 
> > Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
> > Don't top-post http://www.catb.org/jargon/html/T/top-post.html
> > Plain text mails only, please.
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] tpm: Do not dereference NULL pointer if acpi_os_map_memory() fails.
  2012-08-15 20:15   ` Kent Yoder
@ 2012-08-15 22:02     ` Jesper Juhl
  2012-08-15 22:16       ` Jesper Juhl
  0 siblings, 1 reply; 6+ messages in thread
From: Jesper Juhl @ 2012-08-15 22:02 UTC (permalink / raw)
  To: Kent Yoder
  Cc: linux-kernel, tpmdd-devel, Sirrix AG, Marcel Selhorst,
	Rajiv Andrade, Seiji Munetoh, Stefan Berger, Reiner Sailer,
	Kylene Hall

On Wed, 15 Aug 2012, Kent Yoder wrote:

> Hi Jesper,
> 
> > > Unfortunately we just get NULL back, so we can't really tell the user
> > > exactely what went wrong, but we can at least avoid crashing and
> > > return an error (-EIO seemed more generic and more suitable here than
> > > -ENOMEM or something else, so I picked that).
> > 
> > Thanks Jesper. I'd made some updates to tpm_bios.c recently but this
> > should still apply. I'll let you know if not.
> 
>   Of course I'm wrong here, this code moved over into tpm_acpi.c. If you
> can resubmit on top of my staging tree, I will apply it there.
> 

No problem. I'll have that patch for you in a moment when I'm done 
fetching your tree.


> git://github.com/shpedoikal/linux.git v3.6-rc1-tpmdd-staging
> 
> > > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > > ---
> > >  drivers/char/tpm/tpm_bios.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > >   Compile tested only.
> > > 
> > > diff --git a/drivers/char/tpm/tpm_bios.c b/drivers/char/tpm/tpm_bios.c
> > > index 0636520..0c5c274 100644
> > > --- a/drivers/char/tpm/tpm_bios.c
> > > +++ b/drivers/char/tpm/tpm_bios.c
> > > @@ -410,6 +410,11 @@ static int read_log(struct tpm_bios_log *log)
> > >  	log->bios_event_log_end = log->bios_event_log + len;
> > > 
> > >  	virt = acpi_os_map_memory(start, len);
> > > +	if (!virt) {
> 
>   Also please add kfree(log->bios_event_log); in this error path.
> 
Whoops, of course, will do.


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] tpm: Do not dereference NULL pointer if acpi_os_map_memory() fails.
  2012-08-15 22:02     ` Jesper Juhl
@ 2012-08-15 22:16       ` Jesper Juhl
  2012-08-16 15:45         ` Kent Yoder
  0 siblings, 1 reply; 6+ messages in thread
From: Jesper Juhl @ 2012-08-15 22:16 UTC (permalink / raw)
  To: Kent Yoder
  Cc: linux-kernel, tpmdd-devel, Sirrix AG, Marcel Selhorst,
	Rajiv Andrade, Seiji Munetoh, Stefan Berger, Reiner Sailer,
	Kylene Hall

In drivers/char/tpm/tpm_acpi.c::read_log() we call
acpi_os_map_memory(). That call may fail for a number of reasons
(invalid address, out of memory etc). If the call fails it returns
NULL and we just pass that to memcpy() unconditionally, which will go
bad when it tries to dereference the pointer.

Unfortunately we just get NULL back, so we can't really tell the user
exactely what went wrong, but we can at least avoid crashing and
return an error (-EIO seemed more generic and more suitable here than
-ENOMEM or something else, so I picked that).

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/char/tpm/tpm_acpi.c | 5 +++++
 1 file changed, 5 insertions(+)

 note: this patch is against git://github.com/shpedoikal/linux.git v3.6-rc1-tpmdd-staging

diff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
index a1bb5a18..fe3fa94 100644
--- a/drivers/char/tpm/tpm_acpi.c
+++ b/drivers/char/tpm/tpm_acpi.c
@@ -96,6 +96,11 @@ int read_log(struct tpm_bios_log *log)
 	log->bios_event_log_end = log->bios_event_log + len;
 
 	virt = acpi_os_map_memory(start, len);
+	if (!virt) {
+		kfree(log->bios_event_log);
+		printk("%s: ERROR - Unable to map memory\n", __func__);
+		return -EIO;
+	}
 
 	memcpy(log->bios_event_log, virt, len);
 
-- 
1.7.11.4


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] tpm: Do not dereference NULL pointer if acpi_os_map_memory() fails.
  2012-08-15 22:16       ` Jesper Juhl
@ 2012-08-16 15:45         ` Kent Yoder
  0 siblings, 0 replies; 6+ messages in thread
From: Kent Yoder @ 2012-08-16 15:45 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: linux-kernel, tpmdd-devel, Sirrix AG, Marcel Selhorst,
	Rajiv Andrade, Seiji Munetoh, Stefan Berger, Reiner Sailer,
	Kylene Hall

On Thu, Aug 16, 2012 at 12:16:33AM +0200, Jesper Juhl wrote:
> In drivers/char/tpm/tpm_acpi.c::read_log() we call
> acpi_os_map_memory(). That call may fail for a number of reasons
> (invalid address, out of memory etc). If the call fails it returns
> NULL and we just pass that to memcpy() unconditionally, which will go
> bad when it tries to dereference the pointer.
> 
> Unfortunately we just get NULL back, so we can't really tell the user
> exactely what went wrong, but we can at least avoid crashing and
> return an error (-EIO seemed more generic and more suitable here than
> -ENOMEM or something else, so I picked that).

  Thanks Jesper, applied here:

git://github.com/shpedoikal/linux.git v3.6-rc1-tpmdd-staging

Kent

> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
>  drivers/char/tpm/tpm_acpi.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
>  note: this patch is against git://github.com/shpedoikal/linux.git v3.6-rc1-tpmdd-staging
> 
> diff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
> index a1bb5a18..fe3fa94 100644
> --- a/drivers/char/tpm/tpm_acpi.c
> +++ b/drivers/char/tpm/tpm_acpi.c
> @@ -96,6 +96,11 @@ int read_log(struct tpm_bios_log *log)
>  	log->bios_event_log_end = log->bios_event_log + len;
> 
>  	virt = acpi_os_map_memory(start, len);
> +	if (!virt) {
> +		kfree(log->bios_event_log);
> +		printk("%s: ERROR - Unable to map memory\n", __func__);
> +		return -EIO;
> +	}
> 
>  	memcpy(log->bios_event_log, virt, len);
> 
> -- 
> 1.7.11.4
> 
> 
> -- 
> Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
> Don't top-post http://www.catb.org/jargon/html/T/top-post.html
> Plain text mails only, please.
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-08-16 15:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-07 20:50 [PATCH] tpm: Do not dereference NULL pointer if acpi_os_map_memory() fails Jesper Juhl
2012-08-08 18:07 ` Kent Yoder
2012-08-15 20:15   ` Kent Yoder
2012-08-15 22:02     ` Jesper Juhl
2012-08-15 22:16       ` Jesper Juhl
2012-08-16 15:45         ` Kent Yoder

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).