gdiplus: Avoid copying GpImage's busy flag in select_frame_wic().

The 'busy' field in GpImage is used as an atomic variable.  The C11
standard (§5.1.2.4, paragraph 25) states that two conflicting actions to
a memory location shall be both atomic operations, or otherwise properly
synchronized; otherwise, it constitutes a data race.

However, select_frame_wic() performs a non-atomic access to the 'busy'
field on a GpImage that is potentially accessible by other threads.
This happens when select_frame_wic() copies new_image to the old image
object.  Although it does attempt to preserve the value of the 'busy'
field by setting new_image->busy = image->busy first, thereby
effectively assigning an identical value to the field, it is unclear
that this does not actually constitute a theoretical, if not practical,
data race.  This also prevents replacing the busy flag with a mutex or
other synchronization primitives.

Therefore, skip the 'busy' field when copying fields from the new image
to the original image object.
This commit is contained in:
Jinoh Kang 2022-10-22 21:23:18 +09:00 committed by Alexandre Julliard
parent d6b4321125
commit 7abca9742a

View file

@ -3799,6 +3799,7 @@ static GpStatus decode_image_wic(IStream *stream, REFGUID container,
static GpStatus select_frame_wic(GpImage *image, UINT active_frame) static GpStatus select_frame_wic(GpImage *image, UINT active_frame)
{ {
size_t obj_size, body_offset;
GpImage *new_image; GpImage *new_image;
GpStatus status; GpStatus status;
@ -3806,15 +3807,24 @@ static GpStatus select_frame_wic(GpImage *image, UINT active_frame)
if(status != Ok) if(status != Ok)
return status; return status;
new_image->busy = image->busy; if (image->type == ImageTypeBitmap)
obj_size = sizeof(GpBitmap);
else if (image->type == ImageTypeMetafile)
obj_size = sizeof(GpMetafile);
else
{
ERR("unknown image type: %d\n", image->type);
GdipDisposeImage(new_image);
return GenericError;
}
memcpy(&new_image->format, &image->format, sizeof(GUID)); memcpy(&new_image->format, &image->format, sizeof(GUID));
new_image->encoder = image->encoder; new_image->encoder = image->encoder;
image->encoder = NULL; image->encoder = NULL;
free_image_data(image); free_image_data(image);
if (image->type == ImageTypeBitmap) memcpy(image, new_image, FIELD_OFFSET(GpImage, busy));
*(GpBitmap *)image = *(GpBitmap *)new_image; body_offset = RTL_SIZEOF_THROUGH_FIELD(GpImage, busy);
else if (image->type == ImageTypeMetafile) memcpy((char *)image + body_offset, (char *)new_image + body_offset, obj_size - body_offset);
*(GpMetafile *)image = *(GpMetafile *)new_image;
new_image->type = ~0; new_image->type = ~0;
heap_free(new_image); heap_free(new_image);
return Ok; return Ok;