From 20d840e6da15c1574b3ed998bc92f91d1e36c2a5 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Mon, 5 Mar 2018 14:42:09 +0000 Subject: [PATCH] fix a crash with delayed load If a delayed load failed, it could leave the pipeline only half-set up. Sebsequent threads could then segv. Set a load-has-failed flag and test before generate. See https://github.com/jcupitt/libvips/issues/893 --- ChangeLog | 1 + libvips/foreign/foreign.c | 25 +++++++++++++++++++------ libvips/include/vips/foreign.h | 5 +++++ 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index 68f64654..08aaab8c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,6 +4,7 @@ writing twice to memory - better rounding behaviour in convolution means we hit the vector path more often +- fix a crash if a delayed load failed [gsharpsh00ter] 5/1/18 started 8.6.2 - vips_sink_screen() keeps a ref to the input image ... stops a rare race diff --git a/libvips/foreign/foreign.c b/libvips/foreign/foreign.c index 35ad2be5..fb03fd74 100644 --- a/libvips/foreign/foreign.c +++ b/libvips/foreign/foreign.c @@ -18,6 +18,8 @@ * - transform cmyk->rgb if there's an embedded profile * 16/6/17 * - add page_height + * 5/3/18 + * - block _start if one start fails, see #893 */ /* @@ -796,6 +798,11 @@ vips_foreign_load_start( VipsImage *out, void *a, void *b ) VipsForeignLoad *load = VIPS_FOREIGN_LOAD( b ); VipsForeignLoadClass *class = VIPS_FOREIGN_LOAD_GET_CLASS( load ); + /* If this start has failed before in another thread, we can fail now. + */ + if( load->error ) + return( NULL ); + if( !load->real ) { if( !(load->real = vips_foreign_load_temp( load )) ) return( NULL ); @@ -819,19 +826,25 @@ vips_foreign_load_start( VipsImage *out, void *a, void *b ) g_object_set_qdata( G_OBJECT( load->real ), vips__foreign_load_operation, load ); - if( class->load( load ) || - vips_image_pio_input( load->real ) ) - return( NULL ); - - /* ->header() read the header into @out, load has read the + /* Load the image and check the result. + * + * ->header() read the header into @out, load has read the * image into @real. They must match exactly in size, bands, * format and coding for the copy to work. * * Some versions of ImageMagick give different results between * Ping and Load for some formats, for example. + * + * If the load fails, we need to stop */ - if( !vips_foreign_load_iscompat( load->real, out ) ) + if( class->load( load ) || + vips_image_pio_input( load->real ) || + vips_foreign_load_iscompat( load->real, out ) ) { + vips_operation_invalidate( VIPS_OPERATION( load ) ); + load->error = TRUE; + return( NULL ); + } /* We have to tell vips that out depends on real. We've set * the demand hint below, but not given an input there. diff --git a/libvips/include/vips/foreign.h b/libvips/include/vips/foreign.h index 47e285e3..dcbf21c8 100644 --- a/libvips/include/vips/foreign.h +++ b/libvips/include/vips/foreign.h @@ -158,6 +158,11 @@ typedef struct _VipsForeignLoad { * TRUE. */ gboolean disc; + + /* Set if a start function fails. We want to prevent the other starts + * from also triggering the load. + */ + gboolean error; } VipsForeignLoad; typedef struct _VipsForeignLoadClass {