Null pointer dereference?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Null pointer dereference?

Zubin Mevawalla
I was curious if this was a real null pointer dereference issue in
R-devel/src/library/grDevices/src/devPS.c on line 1009?

1000: static type1fontinfo makeType1Font()
1001: {
1002:     type1fontinfo font = (Type1FontInfo *) malloc(sizeof(Type1FontInfo));
1003:     /*
1004:      * Initialise font->metrics.KernPairs to NULL
1005:      * so that we know NOT to free it if we fail to
1006:      * load this font and have to
1007:      * bail out and free this type1fontinfo
1008:      */
1009:     font->metrics.KernPairs = NULL;
1010:     if (!font)
1011: warning(_("failed to allocate Type 1 font info"));
1012:     return font;
1013: }

`font` is conceivably null because there is a null check on line 1010,
but is dereferenced on 1009.

CodeAi, an automated repair tool being developed at Qbit logic,
suggested an if-guard as a fix:

@@ -1006,9 +1006,7 @@ static type1fontinfo makeType1Font()
      * load this font and have to
      * bail out and free this type1fontinfo
      */
-    if(font) {
-        font->metrics.KernPairs = NULL;
-    }
+    font->metrics.KernPairs = NULL;
     if (!font)
        warning(_("failed to allocate Type 1 font info"));
     return font;

Could I submit this as a patch if it looks alright?

Thanks so much,

Zubin

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: Null pointer dereference?

Tomas Kalibera
Thanks, the tool is indeed right, this is a real error. Although it is
unlikely to trigger and unlikely to cause new problems (R would fail
soon anyway if out of memory), it is clearly something to be fixed and
something to be classified as "true positive".

I've fixed this in a way that is consistent with coding style in that file.

Best,
Tomas

On 05/19/2017 06:12 PM, Zubin Mevawalla wrote:

> I was curious if this was a real null pointer dereference issue in
> R-devel/src/library/grDevices/src/devPS.c on line 1009?
>
> 1000: static type1fontinfo makeType1Font()
> 1001: {
> 1002:     type1fontinfo font = (Type1FontInfo *) malloc(sizeof(Type1FontInfo));
> 1003:     /*
> 1004:      * Initialise font->metrics.KernPairs to NULL
> 1005:      * so that we know NOT to free it if we fail to
> 1006:      * load this font and have to
> 1007:      * bail out and free this type1fontinfo
> 1008:      */
> 1009:     font->metrics.KernPairs = NULL;
> 1010:     if (!font)
> 1011: warning(_("failed to allocate Type 1 font info"));
> 1012:     return font;
> 1013: }
>
> `font` is conceivably null because there is a null check on line 1010,
> but is dereferenced on 1009.
>
> CodeAi, an automated repair tool being developed at Qbit logic,
> suggested an if-guard as a fix:
>
> @@ -1006,9 +1006,7 @@ static type1fontinfo makeType1Font()
>        * load this font and have to
>        * bail out and free this type1fontinfo
>        */
> -    if(font) {
> -        font->metrics.KernPairs = NULL;
> -    }
> +    font->metrics.KernPairs = NULL;
>       if (!font)
>          warning(_("failed to allocate Type 1 font info"));
>       return font;
>
> Could I submit this as a patch if it looks alright?
>
> Thanks so much,
>
> Zubin
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel