#1748 potential resource leak in sss_nss_mc_get_record
Closed: Fixed None Opened 11 years ago by jhrozek.

208    /* try max 5 times */
At conditional (4): "count > 0" taking the true branch.
209    for (count = 5; count > 0; count--) {
210        rec = MC_SLOT_TO_PTR(ctx->data_table, slot, struct sss_mc_rec);
211
212        /* fetch record length */
213        b1 = rec->b1;
214        __sync_synchronize();
215        rec_len = rec->len;
216        __sync_synchronize();
217        b2 = rec->b2;
At conditional (5): "!((b1 & 0xff000000U) == 0xf0000000U)" taking the false branch.
At conditional (6): "b1 != b2" taking the false branch.
218        if (!MC_VALID_BARRIER(b1) || b1 != b2) {
219            /* record is inconsistent, retry */
220            continue;
221        }
222
At conditional (7): "rec->len >= 56UL" taking the false branch.
223        if (!MC_CHECK_RECORD_LENGTH(ctx, rec)) {
224            /* record has invalid length */
CID 13125: Resource leak (RESOURCE_LEAK)Variable "copy_rec" going out of scope leaks the storage it points to.
225            return EINVAL;
226        }
227
228        if (rec_len > buf_size) {
229            free(copy_rec);
Calling allocation function "malloc".
Assigning: "copy_rec" = storage returned from "malloc(rec_len)".
230            copy_rec = malloc(rec_len);
At conditional (1): "!copy_rec" taking the false branch.
231            if (!copy_rec) {
232                ret = ENOMEM;
233                goto done;
234            }
235            buf_size = rec_len;
236        }
237        /* we cannot access data directly, we must copy data and then
238         * access the copy */
At conditional (2): "(_b1 & 0xff000000U) == 0xf0000000U" taking the false branch.
Variable "copy_rec" is not freed or pointed-to in function "memcpy".
239        MEMCPY_WITH_BARRIERS(copy_ok, copy_rec, rec, rec_len);
240
241        /* we must check data is consistent again after the copy */
At conditional (3): "copy_ok" taking the false branch.
242        if (copy_ok && b1 == copy_rec->b2) {
243            /* record is consistent, use it */
244            break;
245        }
246    }
247    if (count == 0) {
248        /* couldn't successfully read header we have to give up */
249        ret = EIO;
250        goto done;
251    }

Fields changed

owner: somebody => jhrozek
patch: 0 => 1
status: new => assigned

resolution: => fixed
status: assigned => closed

It was a Coverity bug. No clone is needed.

rhbz: => 0

Metadata Update from @jhrozek:
- Issue assigned to jhrozek
- Issue set to the milestone: NEEDS_TRIAGE

7 years ago

SSSD is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in SSSD's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/SSSD/sssd/issues/2790

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Login to comment on this ticket.

Metadata