RE: [PATCH] bfa: avoid buffer overrun for 12-byte model name
Hi Jim,
Due to BFA_FCS_PORT_SYMBNAME_MODEL_SZ macro value of 12, we are missing some
part of the model name in port/node symbolic name and seeing issues related to
null termination. Mismatch between the actual model size and number of bytes
copied to symbolic name is a bug. Can you please fix this by changing
BFA_FCS_PORT_SYMBNAME_MODEL_SZ to 16 and reduce os_name macro
(BFA_FCS_PORT_SYMBNAME_OSINFO_SZ) to 44, so that both the issues i.e symbolic
name and null termination will be fixed.
Thanks,
Vijay
-Original Message-
From: linux-scsi-ow...@vger.kernel.org
[mailto:linux-scsi-ow...@vger.kernel.org] On Behalf Of Jim Meyering
Sent: Sunday, October 14, 2012 1:51 PM
To: Krishna Gudipati
Cc: Andi Kleen; linux-ker...@vger.kernel.org; James E.J. Bottomley;
linux-scsi@vger.kernel.org
Subject: Re: [PATCH] bfa: avoid buffer overrun for 12-byte model name
Jim Meyering wrote:
Jim Meyering wrote:
Krishna Gudipati wrote:
-Original Message-
From: Jim Meyering [mailto:j...@meyering.net]
Sent: Monday, August 20, 2012 9:55 AM
To: linux-ker...@vger.kernel.org
Cc: Jim Meyering; Jing Huang; Krishna Gudipati; James E.J.
Bottomley; linux- s...@vger.kernel.org
Subject: [PATCH] bfa: avoid buffer overrun for 12-byte model name
From: Jim Meyering meyer...@redhat.com
we use strncpy to copy a model name of length up to 15 (16, if you
count the NUL), into a buffer of size 12 (BFA_FCS_PORT_SYMBNAME_MODEL_SZ).
However, strncpy does not always NUL-terminate, so whenever the
original model string has strlen = 12, the following strncat reads
beyond end of the -
sym_name buffer as it attempts to find end of string.
bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s *fabric) {
bfa_ioc_get_adapter_model(fabric-fcs-bfa-ioc, model);
...
strncpy((char *)port_cfg-sym_name, model,
BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
strncat((char *)port_cfg-sym_name,
BFA_FCS_PORT_SYMBNAME_SEPARATOR,
sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
...
bfa_ioc_get_adapter_model(struct bfa_ioc_s *ioc, char *model) {
struct bfi_ioc_attr_s *ioc_attr;
WARN_ON(!model);
memset((void *)model, 0, BFA_ADAPTER_MODEL_NAME_LEN);
BFA_ADAPTER_MODEL_NAME_LEN = 16
Signed-off-by: Jim Meyering meyer...@redhat.com
---
drivers/scsi/bfa/bfa_fcs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/bfa/bfa_fcs.c
b/drivers/scsi/bfa/bfa_fcs.c index
eaac57e..3329493 100644
--- a/drivers/scsi/bfa/bfa_fcs.c
+++ b/drivers/scsi/bfa/bfa_fcs.c
@@ -713,6 +713,7 @@ bfa_fcs_fabric_psymb_init(struct
bfa_fcs_fabric_s
*fabric)
/* Model name/number */
strncpy((char *)port_cfg-sym_name, model,
BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
+ port_cfg-sym_name[BFA_FCS_PORT_SYMBNAME_MODEL_SZ - 1]
= 0;
strncat((char *)port_cfg-sym_name,
BFA_FCS_PORT_SYMBNAME_SEPARATOR,
sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
Nacked-by: Krishna Gudipati kgudi...@brocade.com
Hi Jim,
This model number is of length 12 bytes and the logic added here
will reset the model last byte.
In addition strncat does not need the src to be null terminated, the
change does not compile even.
NACK to this change.
Hi Krishna,
Thanks for the quick feedback and sorry the patch wasn't quite right.
However, the log is accurate: there is at least a theoretical problem
when the string in model (a buffer of size 16 bytes) has strlen = 12.
While strncat does not require that its second argument be
NUL-terminated, the first one (the destination) must be. Otherwise,
it has no way to determine the end of the string to which it must append the
source bytes.
Ping?
In case it wasn't clear, there *is* a risk of buffer overflow, which
happens when strncpy makes it so strncat's destination is not NUL
terminated.
If you require support for 12-byte model numbers, then you'll have to
increase the length of that buffer
(BFA_FCS_PORT_SYMBNAME_MODEL_SZ) to at least 13.
I've just rebased, and thus confirmed that the patches still apply.
Here is a v2 patch to which I've added the requisite (char*) cast.
However, this whole function is rather unreadable due to the
repetition (12 times!) of (char *)port_cfg-sym_name.
In case someone prefers to factor out that repetition, I've appended
a larger, v3 patch to do that.
Taking Andi's advice, I've made the offending code use strlcpy in place of
strncpy. More importantly, I've fixed the same bug also in the following,
nearly identical function.
-- 8 --
Two functions have this problem:
bfa_fcs_fabric_psymb_init
bfa_fcs_fabric_nsymb_init
They use strncpy to copy a model name of length up to 15 (16, if you count the
NUL), into a buffer of size 12 (BFA_FCS_PORT_SYMBNAME_MODEL_SZ).
However, strncpy does not always NUL-terminate, so whenever the original model
string has strlen = 12, the following strncat reads beyond end of the
-sym_name buffer as it attempts to find end of string.
Instead, use strlcpy, which
Re: [PATCH] bfa: avoid buffer overrun for 12-byte model name
Jim Meyering wrote:
Krishna Gudipati wrote:
-Original Message-
From: Jim Meyering [mailto:j...@meyering.net]
Sent: Monday, August 20, 2012 9:55 AM
To: linux-ker...@vger.kernel.org
Cc: Jim Meyering; Jing Huang; Krishna Gudipati; James E.J. Bottomley; linux-
s...@vger.kernel.org
Subject: [PATCH] bfa: avoid buffer overrun for 12-byte model name
From: Jim Meyering meyer...@redhat.com
we use strncpy to copy a model name of length up to 15 (16, if you count the
NUL), into a buffer of size 12 (BFA_FCS_PORT_SYMBNAME_MODEL_SZ).
However, strncpy does not always NUL-terminate, so whenever the original
model string has strlen = 12, the following strncat reads beyond end of
the -
sym_name buffer as it attempts to find end of string.
bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s *fabric) {
bfa_ioc_get_adapter_model(fabric-fcs-bfa-ioc, model);
...
strncpy((char *)port_cfg-sym_name, model,
BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
strncat((char *)port_cfg-sym_name,
BFA_FCS_PORT_SYMBNAME_SEPARATOR,
sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
...
bfa_ioc_get_adapter_model(struct bfa_ioc_s *ioc, char *model) {
struct bfi_ioc_attr_s *ioc_attr;
WARN_ON(!model);
memset((void *)model, 0, BFA_ADAPTER_MODEL_NAME_LEN);
BFA_ADAPTER_MODEL_NAME_LEN = 16
Signed-off-by: Jim Meyering meyer...@redhat.com
---
drivers/scsi/bfa/bfa_fcs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/bfa/bfa_fcs.c b/drivers/scsi/bfa/bfa_fcs.c index
eaac57e..3329493 100644
--- a/drivers/scsi/bfa/bfa_fcs.c
+++ b/drivers/scsi/bfa/bfa_fcs.c
@@ -713,6 +713,7 @@ bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s
*fabric)
/* Model name/number */
strncpy((char *)port_cfg-sym_name, model,
BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
+ port_cfg-sym_name[BFA_FCS_PORT_SYMBNAME_MODEL_SZ - 1]
= 0;
strncat((char *)port_cfg-sym_name,
BFA_FCS_PORT_SYMBNAME_SEPARATOR,
sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
Nacked-by: Krishna Gudipati kgudi...@brocade.com
Hi Jim,
This model number is of length 12 bytes and the logic added here will
reset the model last byte.
In addition strncat does not need the src to be null terminated, the
change does not compile even.
NACK to this change.
Hi Krishna,
Thanks for the quick feedback and sorry the patch wasn't quite right.
However, the log is accurate: there is at least a theoretical problem
when the string in model (a buffer of size 16 bytes) has strlen = 12.
While strncat does not require that its second argument be NUL-terminated,
the first one (the destination) must be. Otherwise, it has no way to
determine the end of the string to which it must append the source bytes.
Ping?
In case it wasn't clear, there *is* a risk of buffer overflow,
which happens when strncpy makes it so strncat's destination
is not NUL terminated.
If you require support for 12-byte model numbers, then
you'll have to increase the length of that buffer
(BFA_FCS_PORT_SYMBNAME_MODEL_SZ) to at least 13.
I've just rebased, and thus confirmed that the patches still apply.
Here is a v2 patch to which I've added the requisite (char*) cast.
However, this whole function is rather unreadable due to the
repetition (12 times!) of (char *)port_cfg-sym_name.
In case someone prefers to factor out that repetition,
I've appended a larger, v3 patch to do that.
From 4d1ce4e5caf8a5041e5c4f3ae4deddb79c9e247c Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Sun, 29 Apr 2012 10:41:05 +0200
Subject: [PATCHv2] bfa: avoid buffer overrun for 12-byte model name
we use strncpy to copy a model name of length up to 15 (16, if you count
the NUL), into a buffer of size 12 (BFA_FCS_PORT_SYMBNAME_MODEL_SZ).
However, strncpy does not always NUL-terminate, so whenever the original
model string has strlen = 12, the following strncat reads beyond end
of the -sym_name buffer as it attempts to find end of string.
bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s *fabric)
{
bfa_ioc_get_adapter_model(fabric-fcs-bfa-ioc, model);
...
strncpy((char *)port_cfg-sym_name, model,
BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
strncat((char *)port_cfg-sym_name, BFA_FCS_PORT_SYMBNAME_SEPARATOR,
sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
...
bfa_ioc_get_adapter_model(struct bfa_ioc_s *ioc, char *model)
{
struct bfi_ioc_attr_s *ioc_attr;
WARN_ON(!model);
memset((void *)model, 0, BFA_ADAPTER_MODEL_NAME_LEN);
BFA_ADAPTER_MODEL_NAME_LEN = 16
Signed-off-by: Jim Meyering meyer...@redhat.com
---
drivers/scsi/bfa/bfa_fcs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/bfa/bfa_fcs.c b/drivers/scsi/bfa/bfa_fcs.c
index eaac57e..242c37f 100644
--- a/drivers/scsi/bfa/bfa_fcs.c
+++ b/drivers/scsi/bfa/bfa_fcs.c
@@ -713,6 +713,7 @@ bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s *fabric)
/* Model
Re: [PATCH] bfa: avoid buffer overrun for 12-byte model name
Jim Meyering wrote:
Jim Meyering wrote:
Krishna Gudipati wrote:
-Original Message-
From: Jim Meyering [mailto:j...@meyering.net]
Sent: Monday, August 20, 2012 9:55 AM
To: linux-ker...@vger.kernel.org
Cc: Jim Meyering; Jing Huang; Krishna Gudipati; James E.J. Bottomley;
linux-
s...@vger.kernel.org
Subject: [PATCH] bfa: avoid buffer overrun for 12-byte model name
From: Jim Meyering meyer...@redhat.com
we use strncpy to copy a model name of length up to 15 (16, if you count
the
NUL), into a buffer of size 12 (BFA_FCS_PORT_SYMBNAME_MODEL_SZ).
However, strncpy does not always NUL-terminate, so whenever the original
model string has strlen = 12, the following strncat reads beyond end of
the -
sym_name buffer as it attempts to find end of string.
bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s *fabric) {
bfa_ioc_get_adapter_model(fabric-fcs-bfa-ioc, model);
...
strncpy((char *)port_cfg-sym_name, model,
BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
strncat((char *)port_cfg-sym_name,
BFA_FCS_PORT_SYMBNAME_SEPARATOR,
sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
...
bfa_ioc_get_adapter_model(struct bfa_ioc_s *ioc, char *model) {
struct bfi_ioc_attr_s *ioc_attr;
WARN_ON(!model);
memset((void *)model, 0, BFA_ADAPTER_MODEL_NAME_LEN);
BFA_ADAPTER_MODEL_NAME_LEN = 16
Signed-off-by: Jim Meyering meyer...@redhat.com
---
drivers/scsi/bfa/bfa_fcs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/bfa/bfa_fcs.c b/drivers/scsi/bfa/bfa_fcs.c index
eaac57e..3329493 100644
--- a/drivers/scsi/bfa/bfa_fcs.c
+++ b/drivers/scsi/bfa/bfa_fcs.c
@@ -713,6 +713,7 @@ bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s
*fabric)
/* Model name/number */
strncpy((char *)port_cfg-sym_name, model,
BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
+ port_cfg-sym_name[BFA_FCS_PORT_SYMBNAME_MODEL_SZ - 1]
= 0;
strncat((char *)port_cfg-sym_name,
BFA_FCS_PORT_SYMBNAME_SEPARATOR,
sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
Nacked-by: Krishna Gudipati kgudi...@brocade.com
Hi Jim,
This model number is of length 12 bytes and the logic added here will
reset the model last byte.
In addition strncat does not need the src to be null terminated, the
change does not compile even.
NACK to this change.
Hi Krishna,
Thanks for the quick feedback and sorry the patch wasn't quite right.
However, the log is accurate: there is at least a theoretical problem
when the string in model (a buffer of size 16 bytes) has strlen = 12.
While strncat does not require that its second argument be NUL-terminated,
the first one (the destination) must be. Otherwise, it has no way to
determine the end of the string to which it must append the source bytes.
Ping?
In case it wasn't clear, there *is* a risk of buffer overflow,
which happens when strncpy makes it so strncat's destination
is not NUL terminated.
If you require support for 12-byte model numbers, then
you'll have to increase the length of that buffer
(BFA_FCS_PORT_SYMBNAME_MODEL_SZ) to at least 13.
I've just rebased, and thus confirmed that the patches still apply.
Here is a v2 patch to which I've added the requisite (char*) cast.
However, this whole function is rather unreadable due to the
repetition (12 times!) of (char *)port_cfg-sym_name.
In case someone prefers to factor out that repetition,
I've appended a larger, v3 patch to do that.
Taking Andi's advice, I've made the offending code use
strlcpy in place of strncpy. More importantly, I've fixed
the same bug also in the following, nearly identical function.
-- 8 --
Two functions have this problem:
bfa_fcs_fabric_psymb_init
bfa_fcs_fabric_nsymb_init
They use strncpy to copy a model name of length up to 15 (16, if you
count the NUL), into a buffer of size 12 (BFA_FCS_PORT_SYMBNAME_MODEL_SZ).
However, strncpy does not always NUL-terminate, so whenever the original
model string has strlen = 12, the following strncat reads beyond end
of the -sym_name buffer as it attempts to find end of string.
Instead, use strlcpy, which does guarantee NUL-termination.
bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s *fabric)
{
bfa_ioc_get_adapter_model(fabric-fcs-bfa-ioc, model);
...
strncpy((char *)port_cfg-sym_name, model,
BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
strncat((char *)port_cfg-sym_name, BFA_FCS_PORT_SYMBNAME_SEPARATOR,
sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
...
bfa_ioc_get_adapter_model(struct bfa_ioc_s *ioc, char *model)
{
struct bfi_ioc_attr_s *ioc_attr;
WARN_ON(!model);
memset((void *)model, 0, BFA_ADAPTER_MODEL_NAME_LEN);
BFA_ADAPTER_MODEL_NAME_LEN = 16
Signed-off-by: Jim Meyering j...@meyering.net
---
drivers/scsi/bfa/bfa_fcs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/bfa/bfa_fcs.c b/drivers/scsi/bfa/bfa_fcs.c
index d428808..c7f476c 100644
---
RE: [PATCH] bfa: avoid buffer overrun for 12-byte model name
-Original Message-
From: Jim Meyering [mailto:j...@meyering.net]
Sent: Monday, August 20, 2012 9:55 AM
To: linux-ker...@vger.kernel.org
Cc: Jim Meyering; Jing Huang; Krishna Gudipati; James E.J. Bottomley; linux-
s...@vger.kernel.org
Subject: [PATCH] bfa: avoid buffer overrun for 12-byte model name
From: Jim Meyering meyer...@redhat.com
we use strncpy to copy a model name of length up to 15 (16, if you count the
NUL), into a buffer of size 12 (BFA_FCS_PORT_SYMBNAME_MODEL_SZ).
However, strncpy does not always NUL-terminate, so whenever the original
model string has strlen = 12, the following strncat reads beyond end of the -
sym_name buffer as it attempts to find end of string.
bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s *fabric) {
bfa_ioc_get_adapter_model(fabric-fcs-bfa-ioc, model);
...
strncpy((char *)port_cfg-sym_name, model,
BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
strncat((char *)port_cfg-sym_name,
BFA_FCS_PORT_SYMBNAME_SEPARATOR,
sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
...
bfa_ioc_get_adapter_model(struct bfa_ioc_s *ioc, char *model) {
struct bfi_ioc_attr_s *ioc_attr;
WARN_ON(!model);
memset((void *)model, 0, BFA_ADAPTER_MODEL_NAME_LEN);
BFA_ADAPTER_MODEL_NAME_LEN = 16
Signed-off-by: Jim Meyering meyer...@redhat.com
---
drivers/scsi/bfa/bfa_fcs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/bfa/bfa_fcs.c b/drivers/scsi/bfa/bfa_fcs.c index
eaac57e..3329493 100644
--- a/drivers/scsi/bfa/bfa_fcs.c
+++ b/drivers/scsi/bfa/bfa_fcs.c
@@ -713,6 +713,7 @@ bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s
*fabric)
/* Model name/number */
strncpy((char *)port_cfg-sym_name, model,
BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
+ port_cfg-sym_name[BFA_FCS_PORT_SYMBNAME_MODEL_SZ - 1]
= 0;
strncat((char *)port_cfg-sym_name,
BFA_FCS_PORT_SYMBNAME_SEPARATOR,
sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
Nacked-by: Krishna Gudipati kgudi...@brocade.com
Hi Jim,
This model number is of length 12 bytes and the logic added here will reset the
model last byte.
In addition strncat does not need the src to be null terminated, the change
does not compile even.
NACK to this change.
Thanks,
Krishna
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bfa: avoid buffer overrun for 12-byte model name
Krishna Gudipati wrote:
-Original Message-
From: Jim Meyering [mailto:j...@meyering.net]
Sent: Monday, August 20, 2012 9:55 AM
To: linux-ker...@vger.kernel.org
Cc: Jim Meyering; Jing Huang; Krishna Gudipati; James E.J. Bottomley; linux-
s...@vger.kernel.org
Subject: [PATCH] bfa: avoid buffer overrun for 12-byte model name
From: Jim Meyering meyer...@redhat.com
we use strncpy to copy a model name of length up to 15 (16, if you count the
NUL), into a buffer of size 12 (BFA_FCS_PORT_SYMBNAME_MODEL_SZ).
However, strncpy does not always NUL-terminate, so whenever the original
model string has strlen = 12, the following strncat reads beyond end of the
-
sym_name buffer as it attempts to find end of string.
bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s *fabric) {
bfa_ioc_get_adapter_model(fabric-fcs-bfa-ioc, model);
...
strncpy((char *)port_cfg-sym_name, model,
BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
strncat((char *)port_cfg-sym_name,
BFA_FCS_PORT_SYMBNAME_SEPARATOR,
sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
...
bfa_ioc_get_adapter_model(struct bfa_ioc_s *ioc, char *model) {
struct bfi_ioc_attr_s *ioc_attr;
WARN_ON(!model);
memset((void *)model, 0, BFA_ADAPTER_MODEL_NAME_LEN);
BFA_ADAPTER_MODEL_NAME_LEN = 16
Signed-off-by: Jim Meyering meyer...@redhat.com
---
drivers/scsi/bfa/bfa_fcs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/bfa/bfa_fcs.c b/drivers/scsi/bfa/bfa_fcs.c index
eaac57e..3329493 100644
--- a/drivers/scsi/bfa/bfa_fcs.c
+++ b/drivers/scsi/bfa/bfa_fcs.c
@@ -713,6 +713,7 @@ bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s
*fabric)
/* Model name/number */
strncpy((char *)port_cfg-sym_name, model,
BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
+port_cfg-sym_name[BFA_FCS_PORT_SYMBNAME_MODEL_SZ - 1]
= 0;
strncat((char *)port_cfg-sym_name,
BFA_FCS_PORT_SYMBNAME_SEPARATOR,
sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
Nacked-by: Krishna Gudipati kgudi...@brocade.com
Hi Jim,
This model number is of length 12 bytes and the logic added here will
reset the model last byte.
In addition strncat does not need the src to be null terminated, the
change does not compile even.
NACK to this change.
Hi Krishna,
Thanks for the quick feedback and sorry the patch wasn't quite right.
However, the log is accurate: there is at least a theoretical problem
when the string in model (a buffer of size 16 bytes) has strlen = 12.
While strncat does not require that its second argument be NUL-terminated,
the first one (the destination) must be. Otherwise, it has no way to
determine the end of the string to which it must append the source bytes.
Here is a v2 patch to which I've added the requisite (char*) cast.
However, this whole function is rather unreadable due to the
repetition (12 times!) of (char *)port_cfg-sym_name.
In case someone prefers to factor out that repetition,
I've appended a larger, v3 patch to do that.
From 4d1ce4e5caf8a5041e5c4f3ae4deddb79c9e247c Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Sun, 29 Apr 2012 10:41:05 +0200
Subject: [PATCHv2] bfa: avoid buffer overrun for 12-byte model name
we use strncpy to copy a model name of length up to 15 (16, if you count
the NUL), into a buffer of size 12 (BFA_FCS_PORT_SYMBNAME_MODEL_SZ).
However, strncpy does not always NUL-terminate, so whenever the original
model string has strlen = 12, the following strncat reads beyond end
of the -sym_name buffer as it attempts to find end of string.
bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s *fabric)
{
bfa_ioc_get_adapter_model(fabric-fcs-bfa-ioc, model);
...
strncpy((char *)port_cfg-sym_name, model,
BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
strncat((char *)port_cfg-sym_name, BFA_FCS_PORT_SYMBNAME_SEPARATOR,
sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
...
bfa_ioc_get_adapter_model(struct bfa_ioc_s *ioc, char *model)
{
struct bfi_ioc_attr_s *ioc_attr;
WARN_ON(!model);
memset((void *)model, 0, BFA_ADAPTER_MODEL_NAME_LEN);
BFA_ADAPTER_MODEL_NAME_LEN = 16
Signed-off-by: Jim Meyering meyer...@redhat.com
---
drivers/scsi/bfa/bfa_fcs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/bfa/bfa_fcs.c b/drivers/scsi/bfa/bfa_fcs.c
index eaac57e..242c37f 100644
--- a/drivers/scsi/bfa/bfa_fcs.c
+++ b/drivers/scsi/bfa/bfa_fcs.c
@@ -713,6 +713,7 @@ bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s *fabric)
/* Model name/number */
strncpy((char *)port_cfg-sym_name, model,
BFA_FCS_PORT_SYMBNAME_MODEL_SZ);
+ ((char *)port_cfg-sym_name)[BFA_FCS_PORT_SYMBNAME_MODEL_SZ - 1] = 0;
strncat((char *)port_cfg-sym_name, BFA_FCS_PORT_SYMBNAME_SEPARATOR,
sizeof(BFA_FCS_PORT_SYMBNAME_SEPARATOR));
--
1.7.12
From d7f49a0a2f835ec4808772678fe6c4d595ffa8f5 Mon Sep 17 00:00:00 2001
