As part of helping me understand the libfprint code, I ran scan-build
and fixed one NULL pointer dereference it reported, which after tracing
that back led to another small fix.
0001_fix_NULL_pointer_dereference_in_vfs5001_submit_image.patch
===============================================================
submit_image() in drivers/vfs5001.c checks whether `img` is NULL, but
was missing a return at the end of the conditional block, so it would
still go on to dereference the NULL pointer when setting `img->flags`,
`img->width`, and `img->height`.
This was caught by scan-build and I confirmed that scan-build no longer
reported an error here after the fix.
0002-fpi_img_new_assumes_g_malloc0_succeeds.patch
=================================================
fpi_img_new() in img.c assumes that g_malloc0() always succeeds, for
which there is no guarantee.
So if g_malloc0() fails (returns NULL), fpi_img_new() should abort and
return NULL, should not set `img->length`.
Likewise, fpi_img_new_for_imgdev() should check whether fpi_img_new()
returns NULL, and if it does, fpi_img_new_for_imgdev() should abort and
return NULL, should not set `img->width`, `img->height`.
The next step, of course, is making sure all fpi_img_new(),
fpi_img_new_for_imgdev() consumers check for a NULL return value and do
the right thing. A task I haven't tackled yet.
Please guide me on preferred patch workflow style!
==================================================
If I haven't submitted these patches in an acceptable form, or if you
would like me to do it differently next time, please let me know!
I'm still trying to find my way around here, don't really know the best
approach yet :D
Cheers,
Jason
>From 4d3dddf3d2c11477430392f3cd0aaacc82b17945 Mon Sep 17 00:00:00 2001
From: Jason Gerard DeRose <ja...@system76.com>
Date: Fri, 21 Aug 2015 14:09:10 -0600
Subject: [PATCH 1/2] scan-build: fix NULL pointer dereference in
drivers/vfs5001.c submit_image()
---
libfprint/drivers/vfs5011.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libfprint/drivers/vfs5011.c b/libfprint/drivers/vfs5011.c
index 38a9ef9..390649c 100644
--- a/libfprint/drivers/vfs5011.c
+++ b/libfprint/drivers/vfs5011.c
@@ -535,6 +535,7 @@ void submit_image(struct fpi_ssm *ssm, struct vfs5011_data *data)
if (img == NULL) {
fp_err("Failed to create image");
fpi_ssm_mark_aborted(ssm, -1);
+ return;
}
img->flags = FP_IMG_V_FLIPPED;
--
2.5.0
>From a380556f0e66aeb07335079b297244c60bf15d53 Mon Sep 17 00:00:00 2001
From: Jason Gerard DeRose <ja...@system76.com>
Date: Fri, 21 Aug 2015 14:56:06 -0600
Subject: [PATCH 2/2] img.c: fp_img_new(), fp_img_new_for_imgdev() assume
g_malloc0() succeeds
---
libfprint/img.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/libfprint/img.c b/libfprint/img.c
index 3c91d93..1eba320 100644
--- a/libfprint/img.c
+++ b/libfprint/img.c
@@ -48,6 +48,10 @@
struct fp_img *fpi_img_new(size_t length)
{
struct fp_img *img = g_malloc0(sizeof(*img) + length);
+ if (img == NULL) {
+ fp_err("could not allocate fp_img with length=%zd", length);
+ return NULL;
+ }
fp_dbg("length=%zd", length);
img->length = length;
return img;
@@ -59,6 +63,10 @@ struct fp_img *fpi_img_new_for_imgdev(struct fp_img_dev *imgdev)
int width = imgdrv->img_width;
int height = imgdrv->img_height;
struct fp_img *img = fpi_img_new(width * height);
+ if (img == NULL) {
+ fp_err("fpi_img_new() returned NULL");
+ return NULL;
+ }
img->width = width;
img->height = height;
return img;
--
2.5.0
_______________________________________________
fprint mailing list
fprint@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/fprint
