[Devel] [PATCH] eliminate use after free in AcpiOsGetNextFilename()

[Devel] [PATCH] eliminate use after free in AcpiOsGetNextFilename()

[Dean Nelson]

[-- Attachment #1: Type: text/plain, Size: 1136 bytes --]

AcpiOsGetNextFilename() references temp_str in an unlikely error path, after
having freed the memory allocated for it.

Signed-off-by: Dean Nelson <dnelson(a)redhat.com>
---
 source/os_specific/service_layers/osunixdir.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source/os_specific/service_layers/osunixdir.c b/source/os_specific/service_layers/osunixdir.c
index 02b347d..ef2a924 100644
--- a/source/os_specific/service_layers/osunixdir.c
+++ b/source/os_specific/service_layers/osunixdir.c
@@ -239,14 +239,15 @@ AcpiOsGetNextFilename (
             strcat (temp_str, dir_entry->d_name);
 
             err = stat (temp_str, &temp_stat);
-            free (temp_str);
             if (err == -1)
             {
                 fprintf (stderr,
                     "Cannot stat file (should not happen) - %s\n",
                     temp_str);
+                free (temp_str);
                 return (NULL);
             }
+            free (temp_str);
 
             if ((S_ISDIR (temp_stat.st_mode)
                 && (ExternalInfo->RequestedFileType == REQUEST_DIR_ONLY))

Re: [Devel] [PATCH] eliminate use after free in AcpiOsGetNextFilename()

[Moore, Robert]

[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]

Could you open a bugzilla on this? Thanks.

https://bugs.acpica.org/


> -----Original Message-----
> From: Devel [mailto:devel-bounces(a)acpica.org] On Behalf Of Dean Nelson
> Sent: Tuesday, November 19, 2013 8:41 AM
> To: devel(a)acpica.org
> Subject: [Devel] [PATCH] eliminate use after free in
> AcpiOsGetNextFilename()
> 
> AcpiOsGetNextFilename() references temp_str in an unlikely error path,
> after having freed the memory allocated for it.
> 
> Signed-off-by: Dean Nelson <dnelson(a)redhat.com>
> ---
>  source/os_specific/service_layers/osunixdir.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/source/os_specific/service_layers/osunixdir.c
> b/source/os_specific/service_layers/osunixdir.c
> index 02b347d..ef2a924 100644
> --- a/source/os_specific/service_layers/osunixdir.c
> +++ b/source/os_specific/service_layers/osunixdir.c
> @@ -239,14 +239,15 @@ AcpiOsGetNextFilename (
>              strcat (temp_str, dir_entry->d_name);
> 
>              err = stat (temp_str, &temp_stat);
> -            free (temp_str);
>              if (err == -1)
>              {
>                  fprintf (stderr,
>                      "Cannot stat file (should not happen) - %s\n",
>                      temp_str);
> +                free (temp_str);
>                  return (NULL);
>              }
> +            free (temp_str);
> 
>              if ((S_ISDIR (temp_stat.st_mode)
>                  && (ExternalInfo->RequestedFileType == REQUEST_DIR_ONLY))
> _______________________________________________
> Devel mailing list
> Devel(a)acpica.org
> https://lists.acpica.org/mailman/listinfo/devel

Re: [Devel] [PATCH] eliminate use after free in AcpiOsGetNextFilename()

[Dean Nelson]

[-- Attachment #1: Type: text/plain, Size: 978 bytes --]

On 11/19/2013 10:54 AM, Moore, Robert wrote:
> Could you open a bugzilla on this? Thanks.
>
> https://bugs.acpica.org/

Just did.

Sorry, if I'm not doing things in the manner desired. I'm quite a
novice to acpica. If you prefer that we open BZs and not post patches
to the mailing list, I'm fine with that. Just let me know the protocol
to follow.

I'm pulling acpica-unix2 into RHEL and Coverity Scan (a static analysis
tool for C source code) flagged 164 potential 'defects'. A number are
false-positives, but some are legit. (I've not been through them all,
so I don't know the numbers.)

This was one of the legit ones, though the likelihood of it ever
showing up is about NIL. I posted the patch because it was trivial
and from it I hoped to learn how your process works.

As time allows, I hope to deal with the other legitimately flagged
'defects'. But it will probably be awhile, as there are other pressing
needs to attend to.

Thanks,
Dean