[dpdk-dev] [PATCH] config: remove insecure warnings

[dpdk-dev] [PATCH] config: remove insecure warnings

[Thomas Monjalon]

There was an option CONFIG_RTE_INSECURE_FUNCTION_WARNING (disabled by
default), which prevents from using some libc functions:
sprintf, snprintf, vsnprintf, strcpy, strncpy, strcat, strncat, sscanf,
strtok, strsep and strlen.

It's all about using them at the right place with the right precautions.
However, it is neither really possible nor a good advice to disable them.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 config/common_base                           |  5 --
 doc/api/doxy-api-index.md                    |  1 -
 lib/librte_eal/common/Makefile               |  4 --
 lib/librte_eal/common/include/rte_warnings.h | 84 ----------------------------
 mk/target/generic/rte.vars.mk                |  6 --
 5 files changed, 100 deletions(-)
 delete mode 100644 lib/librte_eal/common/include/rte_warnings.h

diff --git a/config/common_base b/config/common_base
index 652a839..edb6a54 100644
--- a/config/common_base
+++ b/config/common_base
@@ -574,11 +574,6 @@ CONFIG_RTE_LIBRTE_PMD_VHOST=n
 CONFIG_RTE_LIBRTE_XEN_DOM0=n
 
 #
-# Enable warning directives
-#
-CONFIG_RTE_INSECURE_FUNCTION_WARNING=n
-
-#
 # Compile the test application
 #
 CONFIG_RTE_APP_TEST=y
diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
index 6675f96..f3b3727 100644
--- a/doc/api/doxy-api-index.md
+++ b/doc/api/doxy-api-index.md
@@ -139,7 +139,6 @@ There are many libraries, so their headers may be grouped by topics:
   [hexdump]            (@ref rte_hexdump.h),
   [debug]              (@ref rte_debug.h),
   [log]                (@ref rte_log.h),
-  [warnings]           (@ref rte_warnings.h),
   [errno]              (@ref rte_errno.h)
 
 - **misc**:
diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile
index dfd64aa..a92c984 100644
--- a/lib/librte_eal/common/Makefile
+++ b/lib/librte_eal/common/Makefile
@@ -42,10 +42,6 @@ INC += rte_hexdump.h rte_devargs.h rte_dev.h rte_vdev.h
 INC += rte_pci_dev_feature_defs.h rte_pci_dev_features.h
 INC += rte_malloc.h rte_keepalive.h rte_time.h
 
-ifeq ($(CONFIG_RTE_INSECURE_FUNCTION_WARNING),y)
-INC += rte_warnings.h
-endif
-
 GENERIC_INC := rte_atomic.h rte_byteorder.h rte_cycles.h rte_prefetch.h
 GENERIC_INC += rte_spinlock.h rte_memcpy.h rte_cpuflags.h rte_rwlock.h
 # defined in mk/arch/$(RTE_ARCH)/rte.vars.mk
diff --git a/lib/librte_eal/common/include/rte_warnings.h b/lib/librte_eal/common/include/rte_warnings.h
deleted file mode 100644
index 54b545c..0000000
--- a/lib/librte_eal/common/include/rte_warnings.h
+++ /dev/null
@@ -1,84 +0,0 @@
-/*-
- *   BSD LICENSE
- *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
- *   All rights reserved.
- *
- *   Redistribution and use in source and binary forms, with or without
- *   modification, are permitted provided that the following conditions
- *   are met:
- *
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in
- *       the documentation and/or other materials provided with the
- *       distribution.
- *     * Neither the name of Intel Corporation nor the names of its
- *       contributors may be used to endorse or promote products derived
- *       from this software without specific prior written permission.
- *
- *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-/**
- * @file
- * Definitions of warnings for use of various insecure functions
- */
-
-#ifndef _RTE_WARNINGS_H_
-#define _RTE_WARNINGS_H_
-
-#ifdef RTE_INSECURE_FUNCTION_WARNING
-
-/* we need to include all used standard header files so that they appear
- * _before_ we poison the function names.
- */
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <stdarg.h>
-#include <errno.h>
-#ifdef RTE_EXEC_ENV_LINUXAPP
-#include <dirent.h>
-#endif
-
-/* the following function are deemed not fully secure for use e.g. they
- * do not always null-terminate arguments */
-#pragma GCC poison sprintf strtok snprintf vsnprintf
-#pragma GCC poison strlen strcpy strcat
-#pragma GCC poison sscanf
-
-/* other unsafe functions may be implemented as macros so just undef them */
-#ifdef strsep
-#undef strsep
-#else
-#pragma GCC poison strsep
-#endif
-
-#ifdef strncpy
-#undef strncpy
-#else
-#pragma GCC poison strncpy
-#endif
-
-#ifdef strncat
-#undef strncat
-#else
-#pragma GCC poison strncat
-#endif
-
-#endif
-
-#endif /* RTE_WARNINGS_H */
diff --git a/mk/target/generic/rte.vars.mk b/mk/target/generic/rte.vars.mk
index b31e426..5d22a6a 100644
--- a/mk/target/generic/rte.vars.mk
+++ b/mk/target/generic/rte.vars.mk
@@ -125,9 +125,6 @@ LDFLAGS += -L$(RTE_OUTPUT)/lib
 ifeq ($(BUILDING_RTE_SDK),1)
 # building sdk
 CFLAGS += -include $(RTE_OUTPUT)/include/rte_config.h
-ifeq ($(CONFIG_RTE_INSECURE_FUNCTION_WARNING),y)
-CFLAGS += -include rte_warnings.h
-endif
 else
 # if we are building an external application, include SDK's lib and
 # includes too
@@ -136,9 +133,6 @@ ifneq ($(wildcard $(RTE_OUTPUT)/include/rte_config.h),)
 CFLAGS += -include $(RTE_OUTPUT)/include/rte_config.h
 endif
 CFLAGS += -include $(RTE_SDK_BIN)/include/rte_config.h
-ifeq ($(CONFIG_RTE_INSECURE_FUNCTION_WARNING),y)
-CFLAGS += -include rte_warnings.h
-endif
 LDFLAGS += -L$(RTE_SDK_BIN)/lib
 endif
 
-- 
2.7.0

Re: [dpdk-dev] [PATCH] config: remove insecure warnings

[Bruce Richardson]

On Sun, Dec 04, 2016 at 11:17:06PM +0100, Thomas Monjalon wrote:
> There was an option CONFIG_RTE_INSECURE_FUNCTION_WARNING (disabled by
> default), which prevents from using some libc functions:
> sprintf, snprintf, vsnprintf, strcpy, strncpy, strcat, strncat, sscanf,
> strtok, strsep and strlen.
> 
> It's all about using them at the right place with the right precautions.
> However, it is neither really possible nor a good advice to disable them.
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Agreed. That option has been in DPDK a long time and I suspect is never
used.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH] config: remove insecure warnings

[Stephen Hemminger]

On Sun,  4 Dec 2016 23:17:06 +0100
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> There was an option CONFIG_RTE_INSECURE_FUNCTION_WARNING (disabled by
> default), which prevents from using some libc functions:
> sprintf, snprintf, vsnprintf, strcpy, strncpy, strcat, strncat, sscanf,
> strtok, strsep and strlen.
> 
> It's all about using them at the right place with the right precautions.
> However, it is neither really possible nor a good advice to disable them.
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Agreed, this is better done by fortify and other static checkers anyway.

Re: [dpdk-dev] [PATCH] config: remove insecure warnings

[Thomas Monjalon]

2016-12-05 10:27, Bruce Richardson:
> On Sun, Dec 04, 2016 at 11:17:06PM +0100, Thomas Monjalon wrote:
> > There was an option CONFIG_RTE_INSECURE_FUNCTION_WARNING (disabled by
> > default), which prevents from using some libc functions:
> > sprintf, snprintf, vsnprintf, strcpy, strncpy, strcat, strncat, sscanf,
> > strtok, strsep and strlen.
> > 
> > It's all about using them at the right place with the right precautions.
> > However, it is neither really possible nor a good advice to disable them.
> > 
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> 
> Agreed. That option has been in DPDK a long time and I suspect is never
> used.
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Applied