[PATCH net-next 0/3] nfp: improve the new rtsym helpers

[PATCH net-next 0/3] nfp: improve the new rtsym helpers

[Jakub Kicinski]

Hi!

This set fixes a bug in ABS rtsym handling I added in net-next,
it expands the error checking and reporting on the rtsym accesses.

Jakub Kicinski (3):
  nfp: fix readq on absolute RTsyms
  nfp: prefix rtsym error messages with symbol name
  nfp: validate rtsym accesses fall within the symbol

 .../netronome/nfp/nfpcore/nfp_rtsym.c         | 75 +++++++++++++++----
 1 file changed, 60 insertions(+), 15 deletions(-)

-- 
2.17.1

[PATCH net-next 1/3] nfp: fix readq on absolute RTsyms

[Jakub Kicinski]

Return the error and report value through the output param.

Fixes: 640917dd81b6 ("nfp: support access to absolute RTsyms")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Francois H. Theron <francois.theron@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c
index 108ce8c5e68e..4003ed76a49a 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c
@@ -337,8 +337,10 @@ int __nfp_rtsym_readq(struct nfp_cpp *cpp, const struct nfp_rtsym *sym,
 	u64 addr;
 	int err;
 
-	if (sym->type == NFP_RTSYM_TYPE_ABS)
-		return sym->addr;
+	if (sym->type == NFP_RTSYM_TYPE_ABS) {
+		*value = sym->addr;
+		return 0;
+	}
 
 	err = nfp_rtsym_to_dest(cpp, sym, action, token, off, &cpp_id, &addr);
 	if (err)
-- 
2.17.1

[PATCH net-next 2/3] nfp: prefix rtsym error messages with symbol name

[Jakub Kicinski]

For ease of debug preface all error messages with the name
of the symbol which caused them.  Use the same message format
for existing messages while at it.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Francois H. Theron <francois.theron@netronome.com>
---
 .../netronome/nfp/nfpcore/nfp_rtsym.c         | 21 ++++++++++---------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c
index 4003ed76a49a..5e416e14e46a 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c
@@ -237,10 +237,10 @@ u64 nfp_rtsym_size(const struct nfp_rtsym *sym)
 {
 	switch (sym->type) {
 	case NFP_RTSYM_TYPE_NONE:
-		pr_err("rtsym type NONE\n");
+		pr_err("rtsym '%s': type NONE\n", sym->name);
 		return 0;
 	default:
-		pr_warn("Unknown rtsym type: %d\n", sym->type);
+		pr_warn("rtsym '%s': unknown type: %d\n", sym->name, sym->type);
 		/* fall through */
 	case NFP_RTSYM_TYPE_OBJECT:
 	case NFP_RTSYM_TYPE_FUNCTION:
@@ -255,7 +255,8 @@ nfp_rtsym_to_dest(struct nfp_cpp *cpp, const struct nfp_rtsym *sym,
 		  u8 action, u8 token, u64 off, u32 *cpp_id, u64 *addr)
 {
 	if (sym->type != NFP_RTSYM_TYPE_OBJECT) {
-		nfp_err(cpp, "Direct access attempt to non-object rtsym\n");
+		nfp_err(cpp, "rtsym '%s': direct access to non-object rtsym\n",
+			sym->name);
 		return -EINVAL;
 	}
 
@@ -270,8 +271,8 @@ nfp_rtsym_to_dest(struct nfp_cpp *cpp, const struct nfp_rtsym *sym,
 		*cpp_id = NFP_CPP_ISLAND_ID(NFP_CPP_TARGET_MU, action, token,
 					    sym->domain);
 	} else if (sym->target < 0) {
-		nfp_err(cpp, "Unhandled RTsym target encoding: %d\n",
-			sym->target);
+		nfp_err(cpp, "rtsym '%s': unhandled target encoding: %d\n",
+			sym->name, sym->target);
 		return -EINVAL;
 	} else {
 		*cpp_id = NFP_CPP_ISLAND_ID(sym->target, action, token,
@@ -451,7 +452,7 @@ u64 nfp_rtsym_read_le(struct nfp_rtsym_table *rtbl, const char *name,
 		break;
 	default:
 		nfp_err(rtbl->cpp,
-			"rtsym '%s' unsupported or non-scalar size: %lld\n",
+			"rtsym '%s': unsupported or non-scalar size: %lld\n",
 			name, nfp_rtsym_size(sym));
 		err = -EINVAL;
 		break;
@@ -497,7 +498,7 @@ int nfp_rtsym_write_le(struct nfp_rtsym_table *rtbl, const char *name,
 		break;
 	default:
 		nfp_err(rtbl->cpp,
-			"rtsym '%s' unsupported or non-scalar size: %lld\n",
+			"rtsym '%s': unsupported or non-scalar size: %lld\n",
 			name, nfp_rtsym_size(sym));
 		err = -EINVAL;
 		break;
@@ -523,18 +524,18 @@ nfp_rtsym_map(struct nfp_rtsym_table *rtbl, const char *name, const char *id,
 	err = nfp_rtsym_to_dest(rtbl->cpp, sym, NFP_CPP_ACTION_RW, 0, 0,
 				&cpp_id, &addr);
 	if (err) {
-		nfp_err(rtbl->cpp, "Symbol %s mapping failed\n", name);
+		nfp_err(rtbl->cpp, "rtsym '%s': mapping failed\n", name);
 		return (u8 __iomem *)ERR_PTR(err);
 	}
 
 	if (sym->size < min_size) {
-		nfp_err(rtbl->cpp, "Symbol %s too small\n", name);
+		nfp_err(rtbl->cpp, "rtsym '%s': too small\n", name);
 		return (u8 __iomem *)ERR_PTR(-EINVAL);
 	}
 
 	mem = nfp_cpp_map_area(rtbl->cpp, id, cpp_id, addr, sym->size, area);
 	if (IS_ERR(mem)) {
-		nfp_err(rtbl->cpp, "Failed to map symbol %s: %ld\n",
+		nfp_err(rtbl->cpp, "rtysm '%s': failed to map: %ld\n",
 			name, PTR_ERR(mem));
 		return mem;
 	}
-- 
2.17.1

[PATCH net-next 3/3] nfp: validate rtsym accesses fall within the symbol

[Jakub Kicinski]

With the accesses to rtsyms now all going via special helpers
we can easily make sure the driver is not reading past the
end of the symbol.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Francois H. Theron <francois.theron@netronome.com>
---
 .../netronome/nfp/nfpcore/nfp_rtsym.c         | 48 +++++++++++++++++--
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c
index 5e416e14e46a..1ad0a015572e 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c
@@ -39,6 +39,8 @@
  *          Espen Skoglund <espen.skoglund@netronome.com>
  *          Francois H. Theron <francois.theron@netronome.com>
  */
+
+#include <asm/unaligned.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/slab.h>
@@ -285,15 +287,23 @@ nfp_rtsym_to_dest(struct nfp_cpp *cpp, const struct nfp_rtsym *sym,
 int __nfp_rtsym_read(struct nfp_cpp *cpp, const struct nfp_rtsym *sym,
 		     u8 action, u8 token, u64 off, void *buf, size_t len)
 {
+	u64 sym_size = nfp_rtsym_size(sym);
 	u32 cpp_id;
 	u64 addr;
 	int err;
 
+	if (off > sym_size) {
+		nfp_err(cpp, "rtsym '%s': read out of bounds: off: %lld + len: %zd > size: %lld\n",
+			sym->name, off, len, sym_size);
+		return -ENXIO;
+	}
+	len = min_t(size_t, len, sym_size - off);
+
 	if (sym->type == NFP_RTSYM_TYPE_ABS) {
-		__le64 tmp = cpu_to_le64(sym->addr);
+		u8 tmp[8];
 
-		len = min(len, sizeof(tmp));
-		memcpy(buf, &tmp, len);
+		put_unaligned_le64(sym->addr, tmp);
+		memcpy(buf, &tmp[off], len);
 
 		return len;
 	}
@@ -318,6 +328,12 @@ int __nfp_rtsym_readl(struct nfp_cpp *cpp, const struct nfp_rtsym *sym,
 	u64 addr;
 	int err;
 
+	if (off + 4 > nfp_rtsym_size(sym)) {
+		nfp_err(cpp, "rtsym '%s': readl out of bounds: off: %lld + 4 > size: %lld\n",
+			sym->name, off, nfp_rtsym_size(sym));
+		return -ENXIO;
+	}
+
 	err = nfp_rtsym_to_dest(cpp, sym, action, token, off, &cpp_id, &addr);
 	if (err)
 		return err;
@@ -338,6 +354,12 @@ int __nfp_rtsym_readq(struct nfp_cpp *cpp, const struct nfp_rtsym *sym,
 	u64 addr;
 	int err;
 
+	if (off + 8 > nfp_rtsym_size(sym)) {
+		nfp_err(cpp, "rtsym '%s': readq out of bounds: off: %lld + 8 > size: %lld\n",
+			sym->name, off, nfp_rtsym_size(sym));
+		return -ENXIO;
+	}
+
 	if (sym->type == NFP_RTSYM_TYPE_ABS) {
 		*value = sym->addr;
 		return 0;
@@ -359,10 +381,18 @@ int nfp_rtsym_readq(struct nfp_cpp *cpp, const struct nfp_rtsym *sym, u64 off,
 int __nfp_rtsym_write(struct nfp_cpp *cpp, const struct nfp_rtsym *sym,
 		      u8 action, u8 token, u64 off, void *buf, size_t len)
 {
+	u64 sym_size = nfp_rtsym_size(sym);
 	u32 cpp_id;
 	u64 addr;
 	int err;
 
+	if (off > sym_size) {
+		nfp_err(cpp, "rtsym '%s': write out of bounds: off: %lld + len: %zd > size: %lld\n",
+			sym->name, off, len, sym_size);
+		return -ENXIO;
+	}
+	len = min_t(size_t, len, sym_size - off);
+
 	err = nfp_rtsym_to_dest(cpp, sym, action, token, off, &cpp_id, &addr);
 	if (err)
 		return err;
@@ -383,6 +413,12 @@ int __nfp_rtsym_writel(struct nfp_cpp *cpp, const struct nfp_rtsym *sym,
 	u64 addr;
 	int err;
 
+	if (off + 4 > nfp_rtsym_size(sym)) {
+		nfp_err(cpp, "rtsym '%s': writel out of bounds: off: %lld + 4 > size: %lld\n",
+			sym->name, off, nfp_rtsym_size(sym));
+		return -ENXIO;
+	}
+
 	err = nfp_rtsym_to_dest(cpp, sym, action, token, off, &cpp_id, &addr);
 	if (err)
 		return err;
@@ -403,6 +439,12 @@ int __nfp_rtsym_writeq(struct nfp_cpp *cpp, const struct nfp_rtsym *sym,
 	u64 addr;
 	int err;
 
+	if (off + 8 > nfp_rtsym_size(sym)) {
+		nfp_err(cpp, "rtsym '%s': writeq out of bounds: off: %lld + 8 > size: %lld\n",
+			sym->name, off, nfp_rtsym_size(sym));
+		return -ENXIO;
+	}
+
 	err = nfp_rtsym_to_dest(cpp, sym, action, token, off, &cpp_id, &addr);
 	if (err)
 		return err;
-- 
2.17.1

Re: [PATCH net-next 0/3] nfp: improve the new rtsym helpers

[David Miller]

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Tue,  4 Sep 2018 07:37:30 -0700

> This set fixes a bug in ABS rtsym handling I added in net-next,
> it expands the error checking and reporting on the rtsym accesses.

Series applied.