| Message ID | 1459942446-25008-1-git-send-email-george.dunlap@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/04/16 12:34, George Dunlap wrote: > Commit 3fec17d4bb56567d139d7806392f4d8702d3f6a7 introduced a bug where > an empty cdrom would cause target_path to be uninitialized. Initialize > target_path to NULL instead. > > The other option here would have been to set target_path to NULL only > on the LIBXL_DISK_FORMAT_EMPTY path. That would potentially enable > the compiler to catch future uninitialized paths, rather than having > those paths (potentially) dereference a NULL pointer. But given that > a bunch of our compilers failed to catch *this* uninitialized path, > setting it to NULL at declaration seems the safer option for now. > > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: George Dunlap <george.dunlap@citrix.com> This looks like it will work. Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Andrew Cooper writes ("Re: [PATCH 1/1] libxl: Fix uninitialized pointer when passing an empty cdrom"):
> On 06/04/16 12:34, George Dunlap wrote:
> > Commit 3fec17d4bb56567d139d7806392f4d8702d3f6a7 introduced a bug where
> > an empty cdrom would cause target_path to be uninitialized. Initialize
> > target_path to NULL instead.
> >
> > The other option here would have been to set target_path to NULL only
> > on the LIBXL_DISK_FORMAT_EMPTY path. That would potentially enable
> > the compiler to catch future uninitialized paths, rather than having
> > those paths (potentially) dereference a NULL pointer. But given that
> > a bunch of our compilers failed to catch *this* uninitialized path,
> > setting it to NULL at declaration seems the safer option for now.
This reasoning is plausible.
> > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > Signed-off-by: George Dunlap <george.dunlap@citrix.com>
>
> This looks like it will work. Reviewed-by: Andrew Cooper
> <andrew.cooper3@citrix.com>
Thanks. Queued.
Ian.
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 5b79aa2..eac5501 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -1307,7 +1307,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc, libxl__device_disk_dev_number(disks[i].vdev, &disk, &part); const char *format; char *drive; - const char *target_path; + const char *target_path = NULL; int colo_mode; if (dev_number == -1) {
Commit 3fec17d4bb56567d139d7806392f4d8702d3f6a7 introduced a bug where an empty cdrom would cause target_path to be uninitialized. Initialize target_path to NULL instead. The other option here would have been to set target_path to NULL only on the LIBXL_DISK_FORMAT_EMPTY path. That would potentially enable the compiler to catch future uninitialized paths, rather than having those paths (potentially) dereference a NULL pointer. But given that a bunch of our compilers failed to catch *this* uninitialized path, setting it to NULL at declaration seems the safer option for now. Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: George Dunlap <george.dunlap@citrix.com> --- CC: Ian Jackson <ian.jackson@citrix.com> CC: Wei Liu <wei.liu2@citrix.com> CC: Andrew Cooper <andrew.cooper3@citrix.com> --- tools/libxl/libxl_dm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)