Linux-EDAC Archive on lore.kernel.org
 help / color / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: Saar Amar <Saar.Amar@microsoft.com>
Cc: "tony.luck@intel.com" <tony.luck@intel.com>,
	"linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>
Subject: Re: [PATCH] Fix dangling pointer in MCE amd.c
Date: Mon, 12 Aug 2019 00:11:32 +0200
Message-ID: <20190811221132.GA14617@zn.tnic> (raw)
In-Reply-To: <AM6PR83MB0312E97C139EA0A2EC942EE0F1D00@AM6PR83MB0312.EURPRD83.prod.outlook.com>

On Sun, Aug 11, 2019 at 09:32:22PM +0000, Saar Amar wrote:
> From: Saar Amar <saaramar@microsoft.com<mailto:saaramar@microsoft.com>>
> Date: Mon, 6 May 2019 02:22:16
> Subject: [PATCH] Fix dangling pointer in MCE amd.c
> 
> Signed-off-by: Saar Amar saar.amar@microsoft.com<mailto:saar.amar@microsoft.com>
> 
> The functions threshold_create_bank() and allocate_threshold_blocks() could create a dangling pointer in an error flow handling. Make sure to NULL those pointers out in that case.
> ---
> 
> 
> diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
> index 6ea7fdc82f3c..a742697ae44c 100644
> --- a/arch/x86/kernel/cpu/mce/amd.c
> +++ b/arch/x86/kernel/cpu/mce/amd.c
> @@ -1259,8 +1259,12 @@ static int allocate_threshold_blocks(unsigned int cpu, unsigned int bank,
>                 return 0;
> 
>         err = allocate_threshold_blocks(cpu, bank, block, address);
> -       if (err)
> +       if (err) {
> +               if (! per_cpu(threshold_banks, cpu)[bank]->blocks) {

I'd venture a guess you don't mean "!" here.

> +                       per_cpu(threshold_banks, cpu)[bank]->blocks = NULL;
> +               }
>                 goto out_free;
> +       }
> 
>         if (b)
>                 kobject_uevent(&b->kobj, KOBJ_ADD);
> @@ -1356,8 +1360,12 @@ static int threshold_create_bank(unsigned int cpu, unsigned int bank)
>         }
> 
>         err = allocate_threshold_blocks(cpu, bank, 0, msr_ops.misc(bank));
> -       if (!err)
> -               goto out;
> +       if (err) {
> +               per_cpu(threshold_banks, cpu)[bank] = NULL;
> +               nb->bank4 = NULL;
> +               goto out_free;
> +       }
> +       goto out;

Anyway, those pointer cleanups should happen at the respective labels
the error handling flow jumps to.

Then, if you want me to apply a patch of yours, you'd have to send
it to me in an applicable format which means, do not create it on
windows.

For how to create it properly, look at:

https://www.kernel.org/doc/html/latest/process/email-clients.html

and

https://www.kernel.org/doc/html/latest/process/submitting-patches.html

Also, do

$ git log -p arch/x86/kernel/cpu/mce/amd.c

to get an idea how to format your patch and what to put there.

And before you send it, run it through the checkpatch.pl tool and fix
most issues it complains about (apply common sense, of course).

Then, send it to yourself only and try applying it. If it works, then
you're good to do.

Right now it looks like this:

$ ./scripts/checkpatch.pl /tmp/saar.amar.01
WARNING: email address 'Saar Amar saar.amar@microsoft.com<mailto:saar.amar@microsoft.com>' might be better as '"Saar Amar saar.amar@microsoft.com" <mailto:saar.amar@microsoft.com>'
#17: 
Signed-off-by: Saar Amar saar.amar@microsoft.com<mailto:saar.amar@microsoft.com>

WARNING: please, no spaces at the start of a line
#32: FILE: arch/x86/kernel/cpu/mce/amd.c:1262:
+       if (err) {$

WARNING: suspect code indent for conditional statements (7, 15)
#32: FILE: arch/x86/kernel/cpu/mce/amd.c:1262:
+       if (err) {
+               if (! per_cpu(threshold_banks, cpu)[bank]->blocks) {

ERROR: code indent should use tabs where possible
#33: FILE: arch/x86/kernel/cpu/mce/amd.c:1263:
+               if (! per_cpu(threshold_banks, cpu)[bank]->blocks) {$

WARNING: please, no spaces at the start of a line
#33: FILE: arch/x86/kernel/cpu/mce/amd.c:1263:
+               if (! per_cpu(threshold_banks, cpu)[bank]->blocks) {$

WARNING: suspect code indent for conditional statements (15, 23)
#33: FILE: arch/x86/kernel/cpu/mce/amd.c:1263:
+               if (! per_cpu(threshold_banks, cpu)[bank]->blocks) {
+                       per_cpu(threshold_banks, cpu)[bank]->blocks = NULL;

ERROR: space prohibited after that '!' (ctx:BxW)
#33: FILE: arch/x86/kernel/cpu/mce/amd.c:1263:
+               if (! per_cpu(threshold_banks, cpu)[bank]->blocks) {
                    ^

WARNING: braces {} are not necessary for single statement blocks
#33: FILE: arch/x86/kernel/cpu/mce/amd.c:1263:
+               if (! per_cpu(threshold_banks, cpu)[bank]->blocks) {
+                       per_cpu(threshold_banks, cpu)[bank]->blocks = NULL;
+               }

ERROR: code indent should use tabs where possible
#34: FILE: arch/x86/kernel/cpu/mce/amd.c:1264:
+                       per_cpu(threshold_banks, cpu)[bank]->blocks = NULL;$

WARNING: please, no spaces at the start of a line
#34: FILE: arch/x86/kernel/cpu/mce/amd.c:1264:
+                       per_cpu(threshold_banks, cpu)[bank]->blocks = NULL;$

ERROR: code indent should use tabs where possible
#35: FILE: arch/x86/kernel/cpu/mce/amd.c:1265:
+               }$

WARNING: please, no spaces at the start of a line
#35: FILE: arch/x86/kernel/cpu/mce/amd.c:1265:
+               }$

WARNING: please, no spaces at the start of a line
#37: FILE: arch/x86/kernel/cpu/mce/amd.c:1267:
+       }$

WARNING: please, no spaces at the start of a line
#47: FILE: arch/x86/kernel/cpu/mce/amd.c:1363:
+       if (err) {$

WARNING: suspect code indent for conditional statements (7, 15)
#47: FILE: arch/x86/kernel/cpu/mce/amd.c:1363:
+       if (err) {
+               per_cpu(threshold_banks, cpu)[bank] = NULL;

ERROR: code indent should use tabs where possible
#48: FILE: arch/x86/kernel/cpu/mce/amd.c:1364:
+               per_cpu(threshold_banks, cpu)[bank] = NULL;$

WARNING: please, no spaces at the start of a line
#48: FILE: arch/x86/kernel/cpu/mce/amd.c:1364:
+               per_cpu(threshold_banks, cpu)[bank] = NULL;$

ERROR: code indent should use tabs where possible
#49: FILE: arch/x86/kernel/cpu/mce/amd.c:1365:
+               nb->bank4 = NULL;$

WARNING: please, no spaces at the start of a line
#49: FILE: arch/x86/kernel/cpu/mce/amd.c:1365:
+               nb->bank4 = NULL;$

ERROR: code indent should use tabs where possible
#50: FILE: arch/x86/kernel/cpu/mce/amd.c:1366:
+               goto out_free;$

WARNING: please, no spaces at the start of a line
#50: FILE: arch/x86/kernel/cpu/mce/amd.c:1366:
+               goto out_free;$

WARNING: please, no spaces at the start of a line
#51: FILE: arch/x86/kernel/cpu/mce/amd.c:1367:
+       }$

WARNING: please, no spaces at the start of a line
#52: FILE: arch/x86/kernel/cpu/mce/amd.c:1368:
+       goto out;$

WARNING: Missing Signed-off-by: line by nominal patch author 'Saar Amar <saaramar@microsoft.com<mailto:saaramar@microsoft.com>>'

total: 7 errors, 17 warnings, 27 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

NOTE: Whitespace errors detected.
      You may wish to use scripts/cleanpatch or scripts/cleanfile

/tmp/saar.amar.01 has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.
---

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

           reply index

Thread overview: expand[flat|nested]  mbox.gz  Atom feed
 [parent not found: <AM6PR83MB0312E97C139EA0A2EC942EE0F1D00@AM6PR83MB0312.EURPRD83.prod.outlook.com>]

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190811221132.GA14617@zn.tnic \
    --to=bp@alien8.de \
    --cc=Saar.Amar@microsoft.com \
    --cc=linux-edac@vger.kernel.org \
    --cc=tony.luck@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-EDAC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-edac/0 linux-edac/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-edac linux-edac/ https://lore.kernel.org/linux-edac \
		linux-edac@vger.kernel.org linux-edac@archiver.kernel.org
	public-inbox-index linux-edac


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-edac


AGPL code for this site: git clone https://public-inbox.org/ public-inbox