From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f54.google.com (mail-pg0-f54.google.com [74.125.83.54]) by dpdk.org (Postfix) with ESMTP id 394C02C39 for ; Wed, 1 Feb 2017 17:53:53 +0100 (CET) Received: by mail-pg0-f54.google.com with SMTP id 3so51869111pgj.3 for ; Wed, 01 Feb 2017 08:53:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:mime-version :content-transfer-encoding; bh=c9jGJbU3YtEFprfqHT9gSYdnKrVnjzd2DCHmb0gV8+s=; b=wo+Tq/bTLqKRmeKg2YYkXHsWdla5PatcthPGxVUOnFSzx+wPkOgjuqxxS4748OJMuZ EpibhBUXiWQlNWAPQHWp0RcsT8sDH+Q/G55uldw/qmJw37aLtq2VVhNpsIi4lv6kzzGU qEhARqRaBkbdJG/bQksDBcbEhzV5ApNFdKBhbyZGbhE5MO4vzUm+6X1L6E0EZbHPaDm+ rPrgCMBhq20mwgv8PJ1FMtSXLKIDfPogriZFnV0s6GS6Bnn7DuN1eKjeX7d4XbvJutxu GxYyeTadkACJNhX1jl3yH4mXIaojOIWhZ+pD8BFyKHdydTwE6wzCa9WzO37Xrse0sRCT 5Z4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-transfer-encoding; bh=c9jGJbU3YtEFprfqHT9gSYdnKrVnjzd2DCHmb0gV8+s=; b=rv4+kCRM2AT+7tgE1CnUNgS9qXxNqy9Czs4imBhWME6JFO7Gj0Ye9Od6DMo7STxRKl LolDu5EsbS/can1haAC7okrUpEGMJpzpUvUmNtYfb3qdEpoMd4/ebFpcuZPI7Ms9zqQa 130JePEfkDXrgopIl92IVBclq9eB80QZNX0Ld+duQbUjMU0kMMx9NPGqgh+IzsByPiqI nHIp/IWKWj4sBM5CZWG+GK6AG1RRP4stILDO7Qo/2JeVWF/qWM7a5O4OeFdynBeUF8CV oj03sU5eMM68hCjfgMzbyMFZZ+wnQOL8fnNfgdqmqk7uDII39piAtn+l1KAPlJpxJVkh rVRg== X-Gm-Message-State: AIkVDXJTvtBR4K3WgaSi79Y5/KbifbBCwi29pTAhdRb5p4UnT06T9ltfL1ECNemLefcUgg== X-Received: by 10.99.174.4 with SMTP id q4mr4901484pgf.186.1485968032348; Wed, 01 Feb 2017 08:53:52 -0800 (PST) Received: from xeon-e3 (204-195-18-65.wavecable.com. [204.195.18.65]) by smtp.gmail.com with ESMTPSA id j192sm50984488pfc.60.2017.02.01.08.53.51 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 01 Feb 2017 08:53:52 -0800 (PST) Date: Wed, 1 Feb 2017 08:53:50 -0800 From: Stephen Hemminger To: Slawomir Mrozowicz , Declan Doherty q Cc: dev@dpdk.org Message-ID: <20170201085321.7168505a@xeon-e3> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: [dpdk-dev] bugs and glitches in rte_cryptodev_devices_get X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 Feb 2017 16:53:53 -0000 The function rte_cryptodev_devices_get has several issues. I was just going to fix it, but think it need to be explained. One potentially serious one (reported by coverity) is: *** CID 141067: (BAD_COMPARE) /lib/librte_cryptodev/rte_cryptodev.c: 503 in rte_cryptodev_devices_get() 497 && (*devs + i)->attached == 498 RTE_CRYPTODEV_ATTACHED) { 499 500 dev = (*devs + i)->device; 501 502 if (dev) >>> CID 141067: (BAD_COMPARE) >>> Truncating the result of "strncmp" to "unsigned char" may cause it to be misinterpreted as 0. Note that "strncmp" may return an integer besides -1, 0, or 1. 503 cmp = strncmp(dev->driver->name, 504 dev_name, 505 strlen(dev_name)); 506 else 507 cmp = strncmp((*devs + i)->data->name, 508 dev_name, /lib/librte_cryptodev/rte_cryptodev.c: 507 in rte_cryptodev_devices_get() 501 502 if (dev) 503 cmp = strncmp(dev->driver->name, 504 dev_name, 505 strlen(dev_name)); 506 else >>> CID 141067: (BAD_COMPARE) >>> Truncating the result of "strncmp" to "unsigned char" may cause it to be misinterpreted as 0. Note that "strncmp" may return an integer besides -1, 0, or 1. 507 cmp = strncmp((*devs + i)->data->name, 508 dev_name, 509 strlen(dev_name)); 510 511 if (cmp == 0) 512 devices[count++] = (*devs + i)->data->dev_id; But also: 1. Incorrect function signature: * function returns int but never a negative value. should be unsigned. * devices argument is not modified should be const. 2. Original ABI seems short sighted with limit of 256 cryptodevs * this seems like 8 bit mindset, should really use unsigned int instead of uint8_t for number of devices. 3. Wacky indention of the if statement. 4. Make variables local to the block they are used (cmp, dev) 5. Use array instead of pointer: ie. instead of *devs + i use devs[i] The overall code in question is: int rte_cryptodev_devices_get(const char *dev_name, uint8_t *devices, uint8_t nb_devices) { uint8_t i, cmp, count = 0; struct rte_cryptodev **devs = &rte_cryptodev_globals->devs; struct rte_device *dev; for (i = 0; i < rte_cryptodev_globals->max_devs && count < nb_devices; i++) { if ((*devs + i) && (*devs + i)->attached == RTE_CRYPTODEV_ATTACHED) { dev = (*devs + i)->device; if (dev) cmp = strncmp(dev->driver->name, dev_name, strlen(dev_name)); else cmp = strncmp((*devs + i)->data->name, dev_name, strlen(dev_name)); if (cmp == 0) devices[count++] = (*devs + i)->data->dev_id; } } return count; } Please fix it.