Skip to content

Commit 4c66e5d

Browse files
committed
Fix some Coverity-discovered locking issues in the subscriptions code.
1 parent a75c321 commit 4c66e5d

2 files changed

Lines changed: 35 additions & 14 deletions

File tree

scheduler/ipp.c

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3506,12 +3506,17 @@ static int /* O - 1 if OK, 0 if not */
35063506
check_rss_recipient(
35073507
const char *recipient) /* I - Recipient URI */
35083508
{
3509+
int i, /* Looping var */
3510+
scount; /* Number of subscriptions */
35093511
cupsd_subscription_t *sub; /* Current subscription */
35103512

35113513

3512-
for (sub = (cupsd_subscription_t *)cupsArrayFirst(Subscriptions);
3513-
sub;
3514-
sub = (cupsd_subscription_t *)cupsArrayNext(Subscriptions))
3514+
cupsRWLockRead(&SubscriptionsLock);
3515+
3516+
for (i = 0, scount = cupsArrayGetCount(Subscriptions); i < scount; i ++)
3517+
{
3518+
sub = (cupsd_subscription_t *)cupsArrayGetElement(Subscriptions, i);
3519+
35153520
if (sub->recipient)
35163521
{
35173522
/*
@@ -3527,6 +3532,9 @@ check_rss_recipient(
35273532
if (*r1 == *r2)
35283533
return (0);
35293534
}
3535+
}
3536+
3537+
cupsRWUnlock(&SubscriptionsLock);
35303538

35313539
return (1);
35323540
}
@@ -9211,14 +9219,14 @@ renew_subscription(
92119219

92129220
sub->expire = sub->lease ? time(NULL) + sub->lease : 0;
92139221

9214-
cupsRWUnlock(&sub->lock);
9215-
92169222
cupsdMarkDirty(CUPSD_DIRTY_SUBSCRIPTIONS);
92179223

92189224
con->response->request.status.status_code = IPP_STATUS_OK;
92199225

92209226
ippAddInteger(con->response, IPP_TAG_SUBSCRIPTION, IPP_TAG_INTEGER,
92219227
"notify-lease-duration", sub->lease);
9228+
9229+
cupsRWUnlock(&sub->lock);
92229230
}
92239231

92249232

scheduler/subscriptions.c

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -419,19 +419,23 @@ cupsdDeleteAllSubscriptions(void)
419419
cupsd_subscription_t *sub; /* Subscription */
420420

421421

422-
if (!Subscriptions)
423-
return;
424-
425422
cupsRWLockWrite(&SubscriptionsLock);
426423

424+
if (!Subscriptions)
425+
goto done;
426+
427427
for (sub = (cupsd_subscription_t *)cupsArrayFirst(Subscriptions);
428428
sub;
429429
sub = (cupsd_subscription_t *)cupsArrayNext(Subscriptions))
430+
{
430431
cupsdDeleteSubscription(sub, -1);
432+
}
431433

432434
cupsArrayDelete(Subscriptions);
433435
Subscriptions = NULL;
434436

437+
done:
438+
435439
cupsRWUnlock(&SubscriptionsLock);
436440
}
437441

@@ -642,20 +646,19 @@ cupsdExpireSubscriptions(
642646
cupsd_job_t *job) /* I - Job, if any */
643647
{
644648
cupsd_subscription_t *sub; /* Current subscription */
645-
int update; /* Update subscriptions.conf? */
649+
int update = 0; /* Update subscriptions.conf? */
646650
time_t curtime; /* Current time */
647651

648652

653+
cupsRWLockWrite(&SubscriptionsLock);
654+
649655
if (cupsArrayCount(Subscriptions) == 0)
650-
return;
656+
goto done;
651657

652658
curtime = time(NULL);
653-
update = 0;
654659

655660
cupsdLogMessage(CUPSD_LOG_DEBUG, "Expiring subscriptions...");
656661

657-
cupsRWLockWrite(&SubscriptionsLock);
658-
659662
for (sub = (cupsd_subscription_t *)cupsArrayFirst(Subscriptions);
660663
sub;
661664
sub = (cupsd_subscription_t *)cupsArrayNext(Subscriptions))
@@ -673,6 +676,8 @@ cupsdExpireSubscriptions(
673676
}
674677
}
675678

679+
done:
680+
676681
cupsRWUnlock(&SubscriptionsLock);
677682

678683
if (update)
@@ -1207,16 +1212,22 @@ cupsdStopAllNotifiers(void)
12071212
* Yes, kill any processes that are left...
12081213
*/
12091214

1215+
cupsRWLockWrite(&SubscriptionsLock);
1216+
12101217
for (sub = (cupsd_subscription_t *)cupsArrayFirst(Subscriptions);
12111218
sub;
12121219
sub = (cupsd_subscription_t *)cupsArrayNext(Subscriptions))
1220+
{
12131221
if (sub->pid)
12141222
{
12151223
cupsdEndProcess(sub->pid, 0);
12161224

12171225
close(sub->pipe);
12181226
sub->pipe = -1;
12191227
}
1228+
}
1229+
1230+
cupsRWUnlock(&SubscriptionsLock);
12201231

12211232
/*
12221233
* Close the status pipes...
@@ -1387,7 +1398,7 @@ cupsd_send_notification(
13871398
cupsdLogMessage(CUPSD_LOG_CRIT,
13881399
"Unable to allocate memory for subscription #%d!",
13891400
sub->id);
1390-
return;
1401+
goto done;
13911402
}
13921403
}
13931404

@@ -1475,6 +1486,8 @@ cupsd_send_notification(
14751486

14761487
sub->next_event_id ++;
14771488

1489+
done:
1490+
14781491
cupsRWUnlock(&sub->lock);
14791492
}
14801493

0 commit comments

Comments
 (0)