[PATCH] Drivers: MarvellYukonDxe driver binding fixes

List overview All Threads
Download

newer

older

[PATCH] Platforms/AMD/Styx: enable...

[PATCH] Platforms/ARM/Juno: fix...

Leif Lindholm

12 Apr 2017 12 Apr '17

8:47 p.m.

Fix a case where MarvellYukonDriverStart attempted to free a buffer before it was allocated.

Fix another case where the function returned without freeing the same buffer. Also correct the error message for that case, which was copied verbatim from the preceding if-statement.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org --- Drivers/Net/MarvellYukonDxe/DriverBinding.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Drivers/Net/MarvellYukonDxe/DriverBinding.c b/Drivers/Net/MarvellYukonDxe/DriverBinding.c index 947d738b10..cbe8ad34e4 100644 --- a/Drivers/Net/MarvellYukonDxe/DriverBinding.c +++ b/Drivers/Net/MarvellYukonDxe/DriverBinding.c @@ -127,7 +127,6 @@ MarvellYukonDriverStart (

if (EFI_ERROR (Status)) { DEBUG ((EFI_D_ERROR, "Marvell Yukon: OpenProtocol: EFI_PCI_IO_PROTOCOL ERROR Status = %r\n", Status)); - gBS->FreePool (YukonDriver); return Status; }

@@ -156,7 +155,8 @@ MarvellYukonDriverStart ( }

if (ScData->msk_if[Port] == NULL) { - DEBUG ((DEBUG_ERROR, "Marvell Yukon: AllocatePool() failed with Status = %r\n", EFI_BAD_BUFFER_SIZE)); + DEBUG ((DEBUG_ERROR, "Marvell Yukon: invalid buffer size\n")); + gBS->FreePool (YukonDriver); return EFI_BAD_BUFFER_SIZE; }

-- 2.11.0

Show replies by date

Alan Ott

12 Apr 12 Apr

9:16 p.m.

New subject: [PATCH] Drivers: MarvellYukonDxe driver binding fixes

Tested-by: Alan Ott alan@softiron.co.uk

On 04/12/2017 04:47 PM, Leif Lindholm wrote:

...

Fix a case where MarvellYukonDriverStart attempted to free a buffer before it was allocated.

Fix another case where the function returned without freeing the same buffer. Also correct the error message for that case, which was copied verbatim from the preceding if-statement.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org

Drivers/Net/MarvellYukonDxe/DriverBinding.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Drivers/Net/MarvellYukonDxe/DriverBinding.c b/Drivers/Net/MarvellYukonDxe/DriverBinding.c index 947d738b10..cbe8ad34e4 100644 --- a/Drivers/Net/MarvellYukonDxe/DriverBinding.c +++ b/Drivers/Net/MarvellYukonDxe/DriverBinding.c @@ -127,7 +127,6 @@ MarvellYukonDriverStart ( if (EFI_ERROR (Status)) { DEBUG ((EFI_D_ERROR, "Marvell Yukon: OpenProtocol: EFI_PCI_IO_PROTOCOL ERROR Status = %r\n", Status));

gBS->FreePool (YukonDriver); return Status; }

@@ -156,7 +155,8 @@ MarvellYukonDriverStart ( } if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: AllocatePool() failed with Status = %r\n", EFI_BAD_BUFFER_SIZE));
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: invalid buffer size\n"));
 gBS->FreePool (YukonDriver);
  return EFI_BAD_BUFFER_SIZE;
}

Daniil Egranov

13 Apr 13 Apr

2:04 a.m.

Tested-by: Daniil Egranov daniil.egranov@arm.com

On 04/12/2017 03:47 PM, Leif Lindholm wrote:

...

Fix a case where MarvellYukonDriverStart attempted to free a buffer before it was allocated.

Fix another case where the function returned without freeing the same buffer. Also correct the error message for that case, which was copied verbatim from the preceding if-statement.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org

Drivers/Net/MarvellYukonDxe/DriverBinding.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Drivers/Net/MarvellYukonDxe/DriverBinding.c b/Drivers/Net/MarvellYukonDxe/DriverBinding.c index 947d738b10..cbe8ad34e4 100644 --- a/Drivers/Net/MarvellYukonDxe/DriverBinding.c +++ b/Drivers/Net/MarvellYukonDxe/DriverBinding.c @@ -127,7 +127,6 @@ MarvellYukonDriverStart ( if (EFI_ERROR (Status)) { DEBUG ((EFI_D_ERROR, "Marvell Yukon: OpenProtocol: EFI_PCI_IO_PROTOCOL ERROR Status = %r\n", Status));

gBS->FreePool (YukonDriver); return Status; }

@@ -156,7 +155,8 @@ MarvellYukonDriverStart ( } if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: AllocatePool() failed with Status = %r\n", EFI_BAD_BUFFER_SIZE));
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: invalid buffer size\n"));
 gBS->FreePool (YukonDriver);
  return EFI_BAD_BUFFER_SIZE;
}

Ard Biesheuvel

7:29 a.m.

New subject: [PATCH] Drivers: MarvellYukonDxe driver binding fixes

On 12 April 2017 at 21:47, Leif Lindholm leif.lindholm@linaro.org wrote:

...

Fix a case where MarvellYukonDriverStart attempted to free a buffer before it was allocated.

Fix another case where the function returned without freeing the same buffer. Also correct the error message for that case, which was copied verbatim from the preceding if-statement.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org

Drivers/Net/MarvellYukonDxe/DriverBinding.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Drivers/Net/MarvellYukonDxe/DriverBinding.c b/Drivers/Net/MarvellYukonDxe/DriverBinding.c index 947d738b10..cbe8ad34e4 100644 --- a/Drivers/Net/MarvellYukonDxe/DriverBinding.c +++ b/Drivers/Net/MarvellYukonDxe/DriverBinding.c @@ -127,7 +127,6 @@ MarvellYukonDriverStart (

if (EFI_ERROR (Status)) { DEBUG ((EFI_D_ERROR, "Marvell Yukon: OpenProtocol: EFI_PCI_IO_PROTOCOL ERROR Status = %r\n", Status));

gBS->FreePool (YukonDriver); return Status; }

@@ -156,7 +155,8 @@ MarvellYukonDriverStart ( }
 if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: AllocatePool() failed with Status = %r\n", EFI_BAD_BUFFER_SIZE));
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: invalid buffer size\n"));
 gBS->FreePool (YukonDriver);
 return EFI_BAD_BUFFER_SIZE;
}

Wouldn't it make much more sense to move this test above the AllocatePool() ?

Leif Lindholm

1:09 p.m.

New subject: [PATCH] Drivers: MarvellYukonDxe driver binding fixes

On Thu, Apr 13, 2017 at 08:29:01AM +0100, Ard Biesheuvel wrote:

...

On 12 April 2017 at 21:47, Leif Lindholm leif.lindholm@linaro.org wrote:

...
Fix a case where MarvellYukonDriverStart attempted to free a buffer before it was allocated.

Fix another case where the function returned without freeing the same buffer. Also correct the error message for that case, which was copied verbatim from the preceding if-statement.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org

Drivers/Net/MarvellYukonDxe/DriverBinding.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Drivers/Net/MarvellYukonDxe/DriverBinding.c b/Drivers/Net/MarvellYukonDxe/DriverBinding.c index 947d738b10..cbe8ad34e4 100644 --- a/Drivers/Net/MarvellYukonDxe/DriverBinding.c +++ b/Drivers/Net/MarvellYukonDxe/DriverBinding.c @@ -127,7 +127,6 @@ MarvellYukonDriverStart (

if (EFI_ERROR (Status)) { DEBUG ((EFI_D_ERROR, "Marvell Yukon: OpenProtocol: EFI_PCI_IO_PROTOCOL ERROR Status = %r\n", Status));

gBS->FreePool (YukonDriver); return Status; }

@@ -156,7 +155,8 @@ MarvellYukonDriverStart ( }
 if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: AllocatePool() failed with Status = %r\n", EFI_BAD_BUFFER_SIZE));
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: invalid buffer size\n"));
 gBS->FreePool (YukonDriver);
 return EFI_BAD_BUFFER_SIZE;
}
Wouldn't it make much more sense to move this test above the AllocatePool() ?

You're not wrong.

How about this?:

...

From 977ad2b9b4ae02c2e1995279bd49c02f86ccd10a Mon Sep 17 00:00:00 2001

From: Leif Lindholm leif.lindholm@linaro.org Date: Wed, 12 Apr 2017 15:23:47 +0100 Subject: [PATCH] Drivers: MarvellYukonDxe driver binding fixes

Fix a case where MarvellYukonDriverStart attempted to free a buffer before it was allocated.

Move another error case which wasn't freeing the same buffer before returning. Also correct the error message for that case, which was copied verbatim from the preceding if-statement.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org --- Drivers/Net/MarvellYukonDxe/DriverBinding.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/Drivers/Net/MarvellYukonDxe/DriverBinding.c b/Drivers/Net/MarvellYukonDxe/DriverBinding.c index 947d738..0abc94e 100644 --- a/Drivers/Net/MarvellYukonDxe/DriverBinding.c +++ b/Drivers/Net/MarvellYukonDxe/DriverBinding.c @@ -127,7 +127,6 @@ MarvellYukonDriverStart (

if (EFI_ERROR (Status)) { DEBUG ((EFI_D_ERROR, "Marvell Yukon: OpenProtocol: EFI_PCI_IO_PROTOCOL ERROR Status = %r\n", Status)); - gBS->FreePool (YukonDriver); return Status; }

@@ -146,6 +145,10 @@ MarvellYukonDriverStart ( }

for (Port = 0; Port < ScData->msk_num_port; Port++) { + if (ScData->msk_if[Port] == NULL) { + DEBUG ((DEBUG_ERROR, "Marvell Yukon: invalid buffer size\n")); + return EFI_BAD_BUFFER_SIZE; + }

Status = gBS->AllocatePool (EfiBootServicesData, sizeof (YUKON_DRIVER), @@ -155,11 +158,6 @@ MarvellYukonDriverStart ( return Status; }

- if (ScData->msk_if[Port] == NULL) { - DEBUG ((DEBUG_ERROR, "Marvell Yukon: AllocatePool() failed with Status = %r\n", EFI_BAD_BUFFER_SIZE)); - return EFI_BAD_BUFFER_SIZE; - } - gBS->SetMem (YukonDriver, sizeof (YUKON_DRIVER), 0); EfiInitializeLock (&YukonDriver->Lock, TPL_NOTIFY);

-- 2.1.4

Ard Biesheuvel

1:13 p.m.

New subject: [PATCH] Drivers: MarvellYukonDxe driver binding fixes

On 13 April 2017 at 14:09, Leif Lindholm leif.lindholm@linaro.org wrote:

...

On Thu, Apr 13, 2017 at 08:29:01AM +0100, Ard Biesheuvel wrote:

...
On 12 April 2017 at 21:47, Leif Lindholm leif.lindholm@linaro.org wrote:

...
Fix a case where MarvellYukonDriverStart attempted to free a buffer before it was allocated.

Fix another case where the function returned without freeing the same buffer. Also correct the error message for that case, which was copied verbatim from the preceding if-statement.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org

Drivers/Net/MarvellYukonDxe/DriverBinding.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Drivers/Net/MarvellYukonDxe/DriverBinding.c b/Drivers/Net/MarvellYukonDxe/DriverBinding.c index 947d738b10..cbe8ad34e4 100644 --- a/Drivers/Net/MarvellYukonDxe/DriverBinding.c +++ b/Drivers/Net/MarvellYukonDxe/DriverBinding.c @@ -127,7 +127,6 @@ MarvellYukonDriverStart (

if (EFI_ERROR (Status)) { DEBUG ((EFI_D_ERROR, "Marvell Yukon: OpenProtocol: EFI_PCI_IO_PROTOCOL ERROR Status = %r\n", Status));

gBS->FreePool (YukonDriver); return Status; }

@@ -156,7 +155,8 @@ MarvellYukonDriverStart ( }
 if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: AllocatePool() failed with Status = %r\n", EFI_BAD_BUFFER_SIZE));
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: invalid buffer size\n"));
 gBS->FreePool (YukonDriver);
 return EFI_BAD_BUFFER_SIZE;
}
Wouldn't it make much more sense to move this test above the AllocatePool() ?
You're not wrong.

How about this?:

From 977ad2b9b4ae02c2e1995279bd49c02f86ccd10a Mon Sep 17 00:00:00 2001 From: Leif Lindholm leif.lindholm@linaro.org Date: Wed, 12 Apr 2017 15:23:47 +0100 Subject: [PATCH] Drivers: MarvellYukonDxe driver binding fixes

Fix a case where MarvellYukonDriverStart attempted to free a buffer before it was allocated.

Move another error case which wasn't freeing the same buffer before returning. Also correct the error message for that case, which was copied verbatim from the preceding if-statement.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org

Drivers/Net/MarvellYukonDxe/DriverBinding.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/Drivers/Net/MarvellYukonDxe/DriverBinding.c b/Drivers/Net/MarvellYukonDxe/DriverBinding.c index 947d738..0abc94e 100644 --- a/Drivers/Net/MarvellYukonDxe/DriverBinding.c +++ b/Drivers/Net/MarvellYukonDxe/DriverBinding.c @@ -127,7 +127,6 @@ MarvellYukonDriverStart (

if (EFI_ERROR (Status)) { DEBUG ((EFI_D_ERROR, "Marvell Yukon: OpenProtocol: EFI_PCI_IO_PROTOCOL ERROR Status = %r\n", Status));

gBS->FreePool (YukonDriver); return Status; }

@@ -146,6 +145,10 @@ MarvellYukonDriverStart ( }

for (Port = 0; Port < ScData->msk_num_port; Port++) {
if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: invalid buffer size\n"));
 return EFI_BAD_BUFFER_SIZE;
}

Status = gBS->AllocatePool (EfiBootServicesData, sizeof (YUKON_DRIVER),
@@ -155,11 +158,6 @@ MarvellYukonDriverStart ( return Status; }
if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: AllocatePool() failed with Status = %r\n", EFI_BAD_BUFFER_SIZE));
 return EFI_BAD_BUFFER_SIZE;
}

gBS->SetMem (YukonDriver, sizeof (YUKON_DRIVER), 0); EfiInitializeLock (&YukonDriver->Lock, TPL_NOTIFY);

Works for me

Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org

Leif Lindholm

1:39 p.m.

New subject: [PATCH] Drivers: MarvellYukonDxe driver binding fixes

Daniil, Alan - can I keep your Tested-by:s?

/ Leif

On Thu, Apr 13, 2017 at 02:13:24PM +0100, Ard Biesheuvel wrote:

...

On 13 April 2017 at 14:09, Leif Lindholm leif.lindholm@linaro.org wrote:

...
On Thu, Apr 13, 2017 at 08:29:01AM +0100, Ard Biesheuvel wrote:

...
On 12 April 2017 at 21:47, Leif Lindholm leif.lindholm@linaro.org wrote:

...
Fix a case where MarvellYukonDriverStart attempted to free a buffer before it was allocated.

Fix another case where the function returned without freeing the same buffer. Also correct the error message for that case, which was copied verbatim from the preceding if-statement.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org

Drivers/Net/MarvellYukonDxe/DriverBinding.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Drivers/Net/MarvellYukonDxe/DriverBinding.c b/Drivers/Net/MarvellYukonDxe/DriverBinding.c index 947d738b10..cbe8ad34e4 100644 --- a/Drivers/Net/MarvellYukonDxe/DriverBinding.c +++ b/Drivers/Net/MarvellYukonDxe/DriverBinding.c @@ -127,7 +127,6 @@ MarvellYukonDriverStart (

if (EFI_ERROR (Status)) { DEBUG ((EFI_D_ERROR, "Marvell Yukon: OpenProtocol: EFI_PCI_IO_PROTOCOL ERROR Status = %r\n", Status));

gBS->FreePool (YukonDriver); return Status; }

@@ -156,7 +155,8 @@ MarvellYukonDriverStart ( }
 if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: AllocatePool() failed with Status = %r\n", EFI_BAD_BUFFER_SIZE));
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: invalid buffer size\n"));
 gBS->FreePool (YukonDriver);
 return EFI_BAD_BUFFER_SIZE;
}
Wouldn't it make much more sense to move this test above the AllocatePool() ?
You're not wrong.

How about this?:

From 977ad2b9b4ae02c2e1995279bd49c02f86ccd10a Mon Sep 17 00:00:00 2001 From: Leif Lindholm leif.lindholm@linaro.org Date: Wed, 12 Apr 2017 15:23:47 +0100 Subject: [PATCH] Drivers: MarvellYukonDxe driver binding fixes

Fix a case where MarvellYukonDriverStart attempted to free a buffer before it was allocated.

Move another error case which wasn't freeing the same buffer before returning. Also correct the error message for that case, which was copied verbatim from the preceding if-statement.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org

Drivers/Net/MarvellYukonDxe/DriverBinding.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/Drivers/Net/MarvellYukonDxe/DriverBinding.c b/Drivers/Net/MarvellYukonDxe/DriverBinding.c index 947d738..0abc94e 100644 --- a/Drivers/Net/MarvellYukonDxe/DriverBinding.c +++ b/Drivers/Net/MarvellYukonDxe/DriverBinding.c @@ -127,7 +127,6 @@ MarvellYukonDriverStart (

if (EFI_ERROR (Status)) { DEBUG ((EFI_D_ERROR, "Marvell Yukon: OpenProtocol: EFI_PCI_IO_PROTOCOL ERROR Status = %r\n", Status));

gBS->FreePool (YukonDriver); return Status; }

@@ -146,6 +145,10 @@ MarvellYukonDriverStart ( }

for (Port = 0; Port < ScData->msk_num_port; Port++) {
if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: invalid buffer size\n"));
 return EFI_BAD_BUFFER_SIZE;
}

Status = gBS->AllocatePool (EfiBootServicesData, sizeof (YUKON_DRIVER),
@@ -155,11 +158,6 @@ MarvellYukonDriverStart ( return Status; }
if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: AllocatePool() failed with Status = %r\n", EFI_BAD_BUFFER_SIZE));
 return EFI_BAD_BUFFER_SIZE;
}

gBS->SetMem (YukonDriver, sizeof (YUKON_DRIVER), 0); EfiInitializeLock (&YukonDriver->Lock, TPL_NOTIFY);
Works for me

Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org

Alan Ott

1:49 p.m.

New subject: [PATCH] Drivers: MarvellYukonDxe driver binding fixes

That's fine.

On 04/13/2017 09:39 AM, Leif Lindholm wrote:

...

Daniil, Alan - can I keep your Tested-by:s?

/ Leif

On Thu, Apr 13, 2017 at 02:13:24PM +0100, Ard Biesheuvel wrote:

...
On 13 April 2017 at 14:09, Leif Lindholm leif.lindholm@linaro.org wrote:

...
On Thu, Apr 13, 2017 at 08:29:01AM +0100, Ard Biesheuvel wrote:

...
On 12 April 2017 at 21:47, Leif Lindholm leif.lindholm@linaro.org wrote:

...
Fix a case where MarvellYukonDriverStart attempted to free a buffer before it was allocated.

Fix another case where the function returned without freeing the same buffer. Also correct the error message for that case, which was copied verbatim from the preceding if-statement.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org

Drivers/Net/MarvellYukonDxe/DriverBinding.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Drivers/Net/MarvellYukonDxe/DriverBinding.c b/Drivers/Net/MarvellYukonDxe/DriverBinding.c index 947d738b10..cbe8ad34e4 100644 --- a/Drivers/Net/MarvellYukonDxe/DriverBinding.c +++ b/Drivers/Net/MarvellYukonDxe/DriverBinding.c @@ -127,7 +127,6 @@ MarvellYukonDriverStart (
if (EFI_ERROR (Status)) {
  DEBUG ((EFI_D_ERROR, "Marvell Yukon: OpenProtocol: EFI_PCI_IO_PROTOCOL ERROR Status = %r\n", Status));
gBS->FreePool (YukonDriver); return Status; }

@@ -156,7 +155,8 @@ MarvellYukonDriverStart ( }
  if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: AllocatePool() failed with Status = %r\n", EFI_BAD_BUFFER_SIZE));
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: invalid buffer size\n"));
 gBS->FreePool (YukonDriver);
  return EFI_BAD_BUFFER_SIZE;
}
Wouldn't it make much more sense to move this test above the AllocatePool() ?
You're not wrong.

How about this?:

From 977ad2b9b4ae02c2e1995279bd49c02f86ccd10a Mon Sep 17 00:00:00 2001 From: Leif Lindholm leif.lindholm@linaro.org Date: Wed, 12 Apr 2017 15:23:47 +0100 Subject: [PATCH] Drivers: MarvellYukonDxe driver binding fixes

Fix a case where MarvellYukonDriverStart attempted to free a buffer before it was allocated.

Move another error case which wasn't freeing the same buffer before returning. Also correct the error message for that case, which was copied verbatim from the preceding if-statement.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org

Drivers/Net/MarvellYukonDxe/DriverBinding.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/Drivers/Net/MarvellYukonDxe/DriverBinding.c b/Drivers/Net/MarvellYukonDxe/DriverBinding.c index 947d738..0abc94e 100644 --- a/Drivers/Net/MarvellYukonDxe/DriverBinding.c +++ b/Drivers/Net/MarvellYukonDxe/DriverBinding.c @@ -127,7 +127,6 @@ MarvellYukonDriverStart (
if (EFI_ERROR (Status)) {
  DEBUG ((EFI_D_ERROR, "Marvell Yukon: OpenProtocol: EFI_PCI_IO_PROTOCOL ERROR Status = %r\n", Status));
gBS->FreePool (YukonDriver); return Status; }

@@ -146,6 +145,10 @@ MarvellYukonDriverStart ( }
for (Port = 0; Port < ScData->msk_num_port; Port++) {
if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: invalid buffer size\n"));
 return EFI_BAD_BUFFER_SIZE;
}

Status = gBS->AllocatePool (EfiBootServicesData, sizeof (YUKON_DRIVER),
@@ -155,11 +158,6 @@ MarvellYukonDriverStart ( return Status; }
if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: AllocatePool() failed with Status = %r\n", EFI_BAD_BUFFER_SIZE));
 return EFI_BAD_BUFFER_SIZE;
}
gBS->SetMem (YukonDriver, sizeof (YUKON_DRIVER), 0);
EfiInitializeLock (&YukonDriver->Lock, TPL_NOTIFY);
Works for me

Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org

Daniil Egranov

2:17 p.m.

Sure.

Thanks, Daniil

On 04/13/2017 08:39 AM, Leif Lindholm wrote:

...

Daniil, Alan - can I keep your Tested-by:s?

/ Leif

On Thu, Apr 13, 2017 at 02:13:24PM +0100, Ard Biesheuvel wrote:

...
On 13 April 2017 at 14:09, Leif Lindholm leif.lindholm@linaro.org wrote:

...
On Thu, Apr 13, 2017 at 08:29:01AM +0100, Ard Biesheuvel wrote:

...
On 12 April 2017 at 21:47, Leif Lindholm leif.lindholm@linaro.org wrote:

...
Fix a case where MarvellYukonDriverStart attempted to free a buffer before it was allocated.

Fix another case where the function returned without freeing the same buffer. Also correct the error message for that case, which was copied verbatim from the preceding if-statement.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org

Drivers/Net/MarvellYukonDxe/DriverBinding.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Drivers/Net/MarvellYukonDxe/DriverBinding.c b/Drivers/Net/MarvellYukonDxe/DriverBinding.c index 947d738b10..cbe8ad34e4 100644 --- a/Drivers/Net/MarvellYukonDxe/DriverBinding.c +++ b/Drivers/Net/MarvellYukonDxe/DriverBinding.c @@ -127,7 +127,6 @@ MarvellYukonDriverStart (
if (EFI_ERROR (Status)) {
  DEBUG ((EFI_D_ERROR, "Marvell Yukon: OpenProtocol: EFI_PCI_IO_PROTOCOL ERROR Status = %r\n", Status));
gBS->FreePool (YukonDriver); return Status; }

@@ -156,7 +155,8 @@ MarvellYukonDriverStart ( }
  if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: AllocatePool() failed with Status = %r\n", EFI_BAD_BUFFER_SIZE));
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: invalid buffer size\n"));
 gBS->FreePool (YukonDriver);
  return EFI_BAD_BUFFER_SIZE;
}
Wouldn't it make much more sense to move this test above the AllocatePool() ?
You're not wrong.

How about this?:

From 977ad2b9b4ae02c2e1995279bd49c02f86ccd10a Mon Sep 17 00:00:00 2001 From: Leif Lindholm leif.lindholm@linaro.org Date: Wed, 12 Apr 2017 15:23:47 +0100 Subject: [PATCH] Drivers: MarvellYukonDxe driver binding fixes

Fix a case where MarvellYukonDriverStart attempted to free a buffer before it was allocated.

Move another error case which wasn't freeing the same buffer before returning. Also correct the error message for that case, which was copied verbatim from the preceding if-statement.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org

Drivers/Net/MarvellYukonDxe/DriverBinding.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/Drivers/Net/MarvellYukonDxe/DriverBinding.c b/Drivers/Net/MarvellYukonDxe/DriverBinding.c index 947d738..0abc94e 100644 --- a/Drivers/Net/MarvellYukonDxe/DriverBinding.c +++ b/Drivers/Net/MarvellYukonDxe/DriverBinding.c @@ -127,7 +127,6 @@ MarvellYukonDriverStart (
if (EFI_ERROR (Status)) {
  DEBUG ((EFI_D_ERROR, "Marvell Yukon: OpenProtocol: EFI_PCI_IO_PROTOCOL ERROR Status = %r\n", Status));
gBS->FreePool (YukonDriver); return Status; }

@@ -146,6 +145,10 @@ MarvellYukonDriverStart ( }
for (Port = 0; Port < ScData->msk_num_port; Port++) {
if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: invalid buffer size\n"));
 return EFI_BAD_BUFFER_SIZE;
}

Status = gBS->AllocatePool (EfiBootServicesData, sizeof (YUKON_DRIVER),
@@ -155,11 +158,6 @@ MarvellYukonDriverStart ( return Status; }
if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: AllocatePool() failed with Status = %r\n", EFI_BAD_BUFFER_SIZE));
 return EFI_BAD_BUFFER_SIZE;
}
gBS->SetMem (YukonDriver, sizeof (YUKON_DRIVER), 0);
EfiInitializeLock (&YukonDriver->Lock, TPL_NOTIFY);
Works for me

Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
Linaro-uefi mailing list Linaro-uefi@lists.linaro.org https://lists.linaro.org/mailman/listinfo/linaro-uefi

Leif Lindholm

2:26 p.m.

Thanks guys, pushed as bfce8b2.

On Thu, Apr 13, 2017 at 09:17:34AM -0500, Daniil Egranov wrote:

...

Sure.

Thanks, Daniil

On 04/13/2017 08:39 AM, Leif Lindholm wrote:

...
Daniil, Alan - can I keep your Tested-by:s?

/ Leif

On Thu, Apr 13, 2017 at 02:13:24PM +0100, Ard Biesheuvel wrote:

...
On 13 April 2017 at 14:09, Leif Lindholm leif.lindholm@linaro.org wrote:

...
On Thu, Apr 13, 2017 at 08:29:01AM +0100, Ard Biesheuvel wrote:

...
On 12 April 2017 at 21:47, Leif Lindholm leif.lindholm@linaro.org wrote:

...
Fix a case where MarvellYukonDriverStart attempted to free a buffer before it was allocated.

Fix another case where the function returned without freeing the same buffer. Also correct the error message for that case, which was copied verbatim from the preceding if-statement.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org

Drivers/Net/MarvellYukonDxe/DriverBinding.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Drivers/Net/MarvellYukonDxe/DriverBinding.c b/Drivers/Net/MarvellYukonDxe/DriverBinding.c index 947d738b10..cbe8ad34e4 100644 --- a/Drivers/Net/MarvellYukonDxe/DriverBinding.c +++ b/Drivers/Net/MarvellYukonDxe/DriverBinding.c @@ -127,7 +127,6 @@ MarvellYukonDriverStart (

if (EFI_ERROR (Status)) { DEBUG ((EFI_D_ERROR, "Marvell Yukon: OpenProtocol: EFI_PCI_IO_PROTOCOL ERROR Status = %r\n", Status));

gBS->FreePool (YukonDriver); return Status; }

@@ -156,7 +155,8 @@ MarvellYukonDriverStart ( }
 if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: AllocatePool() failed with Status = %r\n", EFI_BAD_BUFFER_SIZE));
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: invalid buffer size\n"));
 gBS->FreePool (YukonDriver);
 return EFI_BAD_BUFFER_SIZE;
}
Wouldn't it make much more sense to move this test above the AllocatePool() ?
You're not wrong.

How about this?:

From 977ad2b9b4ae02c2e1995279bd49c02f86ccd10a Mon Sep 17 00:00:00 2001 From: Leif Lindholm leif.lindholm@linaro.org Date: Wed, 12 Apr 2017 15:23:47 +0100 Subject: [PATCH] Drivers: MarvellYukonDxe driver binding fixes

Fix a case where MarvellYukonDriverStart attempted to free a buffer before it was allocated.

Move another error case which wasn't freeing the same buffer before returning. Also correct the error message for that case, which was copied verbatim from the preceding if-statement.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org

Drivers/Net/MarvellYukonDxe/DriverBinding.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/Drivers/Net/MarvellYukonDxe/DriverBinding.c b/Drivers/Net/MarvellYukonDxe/DriverBinding.c index 947d738..0abc94e 100644 --- a/Drivers/Net/MarvellYukonDxe/DriverBinding.c +++ b/Drivers/Net/MarvellYukonDxe/DriverBinding.c @@ -127,7 +127,6 @@ MarvellYukonDriverStart (

if (EFI_ERROR (Status)) { DEBUG ((EFI_D_ERROR, "Marvell Yukon: OpenProtocol: EFI_PCI_IO_PROTOCOL ERROR Status = %r\n", Status));

gBS->FreePool (YukonDriver); return Status; }

@@ -146,6 +145,10 @@ MarvellYukonDriverStart ( }

for (Port = 0; Port < ScData->msk_num_port; Port++) {
if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: invalid buffer size\n"));
 return EFI_BAD_BUFFER_SIZE;
}

Status = gBS->AllocatePool (EfiBootServicesData, sizeof (YUKON_DRIVER),
@@ -155,11 +158,6 @@ MarvellYukonDriverStart ( return Status; }
if (ScData->msk_if[Port] == NULL) {
 DEBUG ((DEBUG_ERROR, "Marvell Yukon: AllocatePool() failed with Status = %r\n", EFI_BAD_BUFFER_SIZE));
 return EFI_BAD_BUFFER_SIZE;
}

gBS->SetMem (YukonDriver, sizeof (YUKON_DRIVER), 0); EfiInitializeLock (&YukonDriver->Lock, TPL_NOTIFY);
Works for me

Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
Linaro-uefi mailing list Linaro-uefi@lists.linaro.org https://lists.linaro.org/mailman/listinfo/linaro-uefi

2810

days inactive

2811

days old

linaro-uefi@lists.linaro.org

9 comments

participants

tags (0)

participants (4)

Alan Ott
Ard Biesheuvel
Daniil Egranov
Leif Lindholm