Project

General

Profile

Actions

Bug #11955

closed

clnt_cots: kmem_free(NULL, 0) is legal

Added by Marcel Telka over 1 year ago. Updated over 1 year ago.

Status:
Closed
Priority:
Low
Assignee:
Category:
nfs - NFS server and client
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Bite-size
Tags:
Gerrit CR:

Description

The code across the clnt_cots.c source file usually directly calls kmem_free(9f) without any previous check for non-NULL buf or non-zero size, for example:

765    kmem_free(p->cku_srcaddr.buf, p->cku_srcaddr.maxlen);
766    kmem_free(p->cku_addr.buf, p->cku_addr.maxlen);

or here:

2196        if (srcaddr->len != destaddr->len) {
2197            kmem_free(srcaddr->buf, srcaddr->maxlen);
2198            srcaddr->buf = kmem_zalloc(destaddr->len, KM_SLEEP);
2199            srcaddr->maxlen = destaddr->len;
2200            srcaddr->len = destaddr->len;
2201            alloc_src = TRUE;
2202        }

But there are still handful places where something like this is done:

1582        if (p->cku_addr.maxlen != 0 && p->cku_addr.buf != NULL)
1583            kmem_free(p->cku_addr.buf, p->cku_addr.maxlen);

or this:

2441            if (srcaddr->maxlen > 0)
2442                kmem_free(srcaddr->buf, srcaddr->maxlen);

or this:

2568    if (cm_entry->x_server.buf != NULL)
2569        kmem_free(cm_entry->x_server.buf, cm_entry->x_server.maxlen);
Such checks before the kmem_free(9f) call are not needed because:
  • kmem_free(NULL, 0) is legal (see kmem_free(9f) man page), and
  • the code always makes sure that when buf != NULL then maxlen != 0, and vice versa, and
  • the code always makes sure that when buf == NULL then maxlen == 0, and vice versa.

Related issues

Related to illumos gate - Bug #11954: rpcmod: Possible memory leak in connmgr_get()ClosedMarcel Telka

Actions
Actions #1

Updated by Marcel Telka over 1 year ago

  • Related to Bug #11954: rpcmod: Possible memory leak in connmgr_get() added
Actions #2

Updated by Marcel Telka over 1 year ago

  • Description updated (diff)
Actions #3

Updated by Marcel Telka over 1 year ago

  • Priority changed from Normal to Low
Actions #5

Updated by Marcel Telka over 1 year ago

  • Status changed from In Progress to Pending RTI
Actions #6

Updated by Electric Monk over 1 year ago

  • Status changed from Pending RTI to Closed
  • % Done changed from 0 to 100

git commit f67d64d998ff666158cc5231b7e80c11c3e922e0

commit  f67d64d998ff666158cc5231b7e80c11c3e922e0
Author: Marcel Telka <marcel@telka.sk>
Date:   2019-11-14T16:19:43.000Z

    11954 rpcmod: Possible memory leak in connmgr_get()
    11955 clnt_cots: kmem_free(NULL, 0) is legal
    Reviewed by: Vitaliy Gusev <gusev.vitaliy@icloud.com>
    Reviewed by: Andy Fiddaman <omnios@citrus-it.co.uk>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions

Also available in: Atom PDF