Skip to content

Fix data races in the cupsEnumDests DNS-SD callbacks #186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
michaelrsweet merged 1 commit into from
Oct 22, 2026
Merged

Fix data races in the cupsEnumDests DNS-SD callbacks #186

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 70 additions & 29 deletions cups/dest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2149,6 +2149,7 @@ cups_dnssd_get_device(
{
_cups_dnssd_device_t key, // Search key
*device; // Device
cups_dnssd_query_t *query = NULL; // Old query to delete
char fullname[1024], // Full name for query
name[128]; // Queue name

Expand All @@ -2160,13 +2161,11 @@ cups_dnssd_get_device(

key.dest.name = name;

cupsRWLockRead(&data->rwlock);
// The device is shared with cups_enum_dests and cups_dest_query_cb, so hold
// the write lock while looking it up and changing it...
cupsRWLockWrite(&data->rwlock);

device = cupsArrayFind(data->devices, &key);

cupsRWUnlock(&data->rwlock);

if (device)
if ((device = cupsArrayFind(data->devices, &key)) != NULL)
{
// Yes, see if we need to do anything with this...
bool update = false; // Non-zero if we need to update
Expand Down Expand Up @@ -2196,7 +2195,7 @@ cups_dnssd_get_device(
if (!update)
{
DEBUG_printf("6cups_dnssd_get_device: No changes to '%s'.", device->dest.name);
return (device);
goto done;
}
}
else
Expand All @@ -2205,25 +2204,23 @@ cups_dnssd_get_device(
DEBUG_printf("6cups_dnssd_get_device: Adding '%s' for %s with domain '%s'.", serviceName, !strcmp(regtype, "_ipps._tcp") ? "IPPS" : "IPP", replyDomain);

if ((device = calloc(sizeof(_cups_dnssd_device_t), 1)) == NULL)
return (NULL);
goto done;

device->dest.name = _cupsStrAlloc(name);
device->domain = _cupsStrAlloc(replyDomain);
device->regtype = _cupsStrAlloc(regtype);

device->dest.num_options = cupsAddOption("printer-info", serviceName, 0, &device->dest.options);

cupsRWLockWrite(&data->rwlock);
cupsArrayAdd(data->devices, device);
cupsRWUnlock(&data->rwlock);
}

// Set the "full name" of this service, which is used for queries...
cupsDNSSDAssembleFullName(fullname, sizeof(fullname), serviceName, regtype, replyDomain);
_cupsStrFree(device->fullname);
device->fullname = _cupsStrAlloc(fullname);

cupsDNSSDQueryDelete(device->query);
query = device->query;
device->query = NULL;

if (device->state == _CUPS_DNSSD_ACTIVE)
Expand All @@ -2233,6 +2230,19 @@ cups_dnssd_get_device(
(*data->cb)(data->user_data, CUPS_DEST_FLAGS_REMOVED, &device->dest);
device->state = _CUPS_DNSSD_NEW;
}
else if (query && (device->state == _CUPS_DNSSD_QUERY || device->state == _CUPS_DNSSD_PENDING))
{
// Query the device again using the new full name...
device->state = _CUPS_DNSSD_NEW;
}

done:

cupsRWUnlock(&data->rwlock);

// Delete the old query after unlocking, since the DNS-SD context may need
// its own lock to do so...
cupsDNSSDQueryDelete(query);

return (device);
}
Expand Down Expand Up @@ -2282,11 +2292,13 @@ cups_dest_query_cb(

dkey.dest.name = name;

cupsRWLockRead(&data->rwlock);
// Hold the write lock while updating the device since cups_enum_dests reads
// it from another thread...
cupsRWLockWrite(&data->rwlock);

device = cupsArrayFind(data->devices, &dkey);
cupsRWUnlock(&data->rwlock);

if (device && device->state == _CUPS_DNSSD_NEW)
if (device && device->state == _CUPS_DNSSD_QUERY)
{
// Found it, pull out the make and model from the TXT record and save it...
const uint8_t *txt, // Pointer into data
Expand Down Expand Up @@ -2519,6 +2531,8 @@ cups_dest_query_cb(
{
DEBUG_printf("6cups_dnssd_query: Ignoring TXT record for '%s'.", fullname);
}

cupsRWUnlock(&data->rwlock);
}


Expand Down Expand Up @@ -2680,6 +2694,7 @@ cups_enum_dests(
double curtime; // Current time
_cups_dnssd_data_t data; // Data for callback
_cups_dnssd_device_t *device; // Current device
cups_array_t *queries = NULL; // Devices to query
cups_dnssd_t *dnssd = NULL; // DNS-SD context
char filename[1024]; // Local lpoptions file
_cups_globals_t *cg = _cupsGlobals(); // Pointer to library globals
Expand Down Expand Up @@ -2912,13 +2927,14 @@ cups_enum_dests(
remaining = msec;

curtime = cupsGetClock();
queries = cupsArrayNew(NULL, NULL, NULL, 0, NULL, NULL);

while (remaining > 0 && (!cancel || !*cancel))
{
// Check for input...
DEBUG_printf("1cups_enum_dests: remaining=%d", remaining);

cupsRWLockRead(&data.rwlock);
cupsRWLockWrite(&data.rwlock);

for (i = 0, num_devices = cupsArrayGetCount(data.devices), count = 0, completed = 0; i < num_devices; i ++)
{
Expand All @@ -2932,18 +2948,9 @@ cups_enum_dests(

if (!device->query && device->state == _CUPS_DNSSD_NEW)
{
DEBUG_printf("1cups_enum_dests: Querying '%s'.", device->fullname);

if ((device->query = cupsDNSSDQueryNew(dnssd, CUPS_DNSSD_IF_INDEX_ANY, device->fullname, CUPS_DNSSD_RRTYPE_TXT, cups_dest_query_cb, &data)) != NULL)
{
count ++;
}
else
{
device->state = _CUPS_DNSSD_ERROR;

DEBUG_puts("1cups_enum_dests: Query failed.");
}
// Start the query after releasing the lock below...
if (cupsArrayAdd(queries, device))
device->state = _CUPS_DNSSD_QUERY;
}
else if (device->query && device->state == _CUPS_DNSSD_PENDING)
{
Expand Down Expand Up @@ -3027,11 +3034,44 @@ cups_enum_dests(
}
}

DEBUG_printf("1cups_enum_dests: remaining=%d, completed=%u, count=%u, devices count=%u", remaining, (unsigned)completed, (unsigned)count, (unsigned)cupsArrayGetCount(data.devices));
DEBUG_printf("1cups_enum_dests: remaining=%d, completed=%u, count=%u, devices count=%u", remaining, (unsigned)completed, (unsigned)count, (unsigned)num_devices);

cupsRWUnlock(&data.rwlock);

if (completed && completed == cupsArrayGetCount(data.devices))
// Start new queries without holding the lock, since the DNS-SD callbacks
// take the lock while the DNS-SD context holds its own lock...
for (device = (_cups_dnssd_device_t *)cupsArrayGetFirst(queries); device && remaining > 0; device = (_cups_dnssd_device_t *)cupsArrayGetNext(queries))
{
char fullname[1024]; // Full name for query
cups_dnssd_query_t *query; // New query

cupsRWLockRead(&data.rwlock);
cupsCopyString(fullname, device->fullname, sizeof(fullname));
cupsRWUnlock(&data.rwlock);

DEBUG_printf("1cups_enum_dests: Querying '%s'.", fullname);

query = cupsDNSSDQueryNew(dnssd, CUPS_DNSSD_IF_INDEX_ANY, fullname, CUPS_DNSSD_RRTYPE_TXT, cups_dest_query_cb, &data);

cupsRWLockWrite(&data.rwlock);

if (query)
{
device->query = query;
}
else
{
DEBUG_puts("1cups_enum_dests: Query failed.");

device->state = _CUPS_DNSSD_ERROR;
}

cupsRWUnlock(&data.rwlock);
}

cupsArrayClear(queries);

if (completed && completed == num_devices)
break;

usleep(100000);
Expand All @@ -3043,6 +3083,7 @@ cups_enum_dests(
enum_finished:

cupsDNSSDDelete(dnssd);
cupsArrayDelete(queries);

cupsFreeDests(data.num_dests, data.dests);
cupsFreeDests(data.num_local, data.local_dests);
Expand Down