Re: [PATCH] tmpfs: fix uninitialized return value in shmem_link

2019-02-28 Thread Nathan Chancellor
On Wed, Feb 27, 2019 at 09:09:40AM -0500, Qian Cai wrote:
> On Mon, 2019-02-25 at 16:07 -0800, Linus Torvalds wrote:
> > On Mon, Feb 25, 2019 at 4:03 PM Qian Cai  wrote:
> > > > 
> > > > Of course, that's just gcc. I have no idea what llvm ends up doing.
> > > 
> > > Clang 7.0:
> > > 
> > > # clang  -O2 -S -Wall /tmp/test.c
> > > /tmp/test.c:46:6: warning: variable 'ret' is used uninitialized whenever
> > > 'if'
> > > condition is false [-Wsometimes-uninitialized]
> > 
> > Ok, good.
> > 
> > Do we have any clang builds in any of the zero-day robot
> > infrastructure or something? Should we?
> > 
> > And maybe this was how Dan noticed the problem in the first place? Or
> > is it just because of his eagle-eyes?
> > 
> 
> BTW, even clang is able to generate warnings in your sample code, it does not
> generate any warnings when compiling the buggy shmem.o via "make CC=clang". 
> Here

Unfortunately, scripts/Kbuild.extrawarn disables -Wuninitialized for
Clang, which also disables -Wsometimes-uninitialized:

https://github.com/ClangBuiltLinux/linux/issues/381
https://clang.llvm.org/docs/DiagnosticsReference.html#wuninitialized

I'm going to be sending out patches to fix the warnings found with it
then enable it going forward so that things like this get caught.

Nathan

> is the objdump for arm64 (with KASAN_SW_TAGS inline).
> 
> effc :
> {
> effc:   f81c0ff7str x23, [sp, #-64]!
> f000:   a90157f6stp x22, x21, [sp, #16]
> f004:   a9024ff4stp x20, x19, [sp, #32]
> f008:   a9037bfdstp x29, x30, [sp, #48]
> f00c:   9100c3fdadd x29, sp, #0x30
> f010:   aa0203f3mov x19, x2
> f014:   aa0103f5mov x21, x1
> f018:   aa0003f4mov x20, x0
> f01c:   9400bl  0 <_mcount>
> f020:   91016280add x0, x20, #0x58
> f024:   d2c20017mov x23, #0x1000//
> #17592186044416
> f028:   b2481c08orr x8, x0, #0xff00
> f02c:   f2fdfff7movkx23, #0xefff, lsl #48
> f030:   d344fd08lsr x8, x8, #4
> f034:   38776909ldrbw9, [x8, x23]
> f038:   940017d5bl  14f8c 
> f03c:   5460b.eqf048   // b.none
> f040:   7103fd1fcmp w8, #0xff
> f044:   54000981b.nef174   // b.any
> f048:   f9400014ldr x20, [x0]
> if (inode->i_nlink) {
> f04c:   91010280add x0, x20, #0x40
> f050:   b2481c08orr x8, x0, #0xff00
> f054:   d344fd08lsr x8, x8, #4
> f058:   38776909ldrbw9, [x8, x23]
> f05c:   940017ccbl  14f8c 
> f060:   5460b.eqf06c   // b.none
> f064:   7103fd1fcmp w8, #0xff
> f068:   540008a1b.nef17c   // b.any
> f06c:   b948ldr w8, [x0]
> f070:   34000148cbz w8, f098 
> f074:   940018fdbl  15468 
> ret = shmem_reserve_inode(inode->i_sb);
> f078:   38776909ldrbw9, [x8, x23]
> f07c:   940017c4bl  14f8c 
> f080:   5460b.eqf08c   // b.none
> f084:   7103fd1fcmp w8, #0xff
> f088:   540007e1b.nef184   // b.any
> f08c:   f940ldr x0, [x0]
> f090:   97fffcf6bl  e468 
> if (ret)
> f094:   35000660cbnzw0, f160 
> dir->i_size += BOGO_DIRENT_SIZE;
> f098:   910122a0add x0, x21, #0x48
> f09c:   b2481c08orr x8, x0, #0xff00
> f0a0:   d344fd09lsr x9, x8, #4
> f0a4:   3877692aldrbw10, [x9, x23]
> f0a8:   94001828bl  15148 
> f0ac:   5460b.eqf0b8   // b.none
> f0b0:   7103fd1fcmp w8, #0xff
> f0b4:   540006c1b.nef18c   // b.any
> f0b8:   38776929ldrbw9, [x9, x23]
> f0bc:   94001a4abl  159e4 
> f0c0:   5460b.eqf0cc   // b.none
> f0c4:   7103fd1fcmp w8, #0xff
> f0c8:   54000661b.nef194   // b.any
> f0cc:   f909str x9, [x0]
> inode->i_ctime = dir->i_ctime = dir->i_mtime = current_time(inode);
> f0d0:   aa1403e0mov x0, x20
> f0d4:   910182b6add x22, x21, #0x60
> f0d8:   9400bl  0 
> f0dc:   b2481ec9orr x9, x22, #0xff00
> f0e0:   d344fd29lsr x9, x9, #4
> 


Re: [PATCH] tmpfs: fix uninitialized return value in shmem_link

2019-02-27 Thread Qian Cai
On Wed, 2019-02-27 at 09:09 -0500, Qian Cai wrote:
> On Mon, 2019-02-25 at 16:07 -0800, Linus Torvalds wrote:
> > On Mon, Feb 25, 2019 at 4:03 PM Qian Cai  wrote:
> > > > 
> > > > Of course, that's just gcc. I have no idea what llvm ends up doing.
> > > 
> > > Clang 7.0:
> > > 
> > > # clang  -O2 -S -Wall /tmp/test.c
> > > /tmp/test.c:46:6: warning: variable 'ret' is used uninitialized whenever
> > > 'if'
> > > condition is false [-Wsometimes-uninitialized]
> > 
> > Ok, good.
> > 
> > Do we have any clang builds in any of the zero-day robot
> > infrastructure or something? Should we?
> > 
> > And maybe this was how Dan noticed the problem in the first place? Or
> > is it just because of his eagle-eyes?
> > 
> 
> BTW, even clang is able to generate warnings in your sample code, it does not
> generate any warnings when compiling the buggy shmem.o via "make CC=clang".
> Here is the objdump for arm64 (with KASAN_SW_TAGS inline).
> 

Ah, thanks to the commit 6e8d666e9253 ("Disable "maybe-uninitialized" warning
globally"), it will no longer generate this type of warnings until using "make
W=1" due to the commit a76bcf557ef4 ("Kbuild: enable -Wmaybe-uninitialized
warning for 'make W=1'"). Anyway, the generated code is the same using clang
with and without this patch.

d_instantiate(dentry, inode);
4eec:   9400bl  0 
ret = shmem_reserve_inode(inode->i_sb);
4ef0:   2a1f03e0mov w0, wzr < ret = 0
return ret;




Re: [PATCH] tmpfs: fix uninitialized return value in shmem_link

2019-02-27 Thread Qian Cai
On Mon, 2019-02-25 at 16:07 -0800, Linus Torvalds wrote:
> On Mon, Feb 25, 2019 at 4:03 PM Qian Cai  wrote:
> > > 
> > > Of course, that's just gcc. I have no idea what llvm ends up doing.
> > 
> > Clang 7.0:
> > 
> > # clang  -O2 -S -Wall /tmp/test.c
> > /tmp/test.c:46:6: warning: variable 'ret' is used uninitialized whenever
> > 'if'
> > condition is false [-Wsometimes-uninitialized]
> 
> Ok, good.
> 
> Do we have any clang builds in any of the zero-day robot
> infrastructure or something? Should we?
> 
> And maybe this was how Dan noticed the problem in the first place? Or
> is it just because of his eagle-eyes?
> 

BTW, even clang is able to generate warnings in your sample code, it does not
generate any warnings when compiling the buggy shmem.o via "make CC=clang". Here
is the objdump for arm64 (with KASAN_SW_TAGS inline).

effc :
{
effc:   f81c0ff7str x23, [sp, #-64]!
f000:   a90157f6stp x22, x21, [sp, #16]
f004:   a9024ff4stp x20, x19, [sp, #32]
f008:   a9037bfdstp x29, x30, [sp, #48]
f00c:   9100c3fdadd x29, sp, #0x30
f010:   aa0203f3mov x19, x2
f014:   aa0103f5mov x21, x1
f018:   aa0003f4mov x20, x0
f01c:   9400bl  0 <_mcount>
f020:   91016280add x0, x20, #0x58
f024:   d2c20017mov x23, #0x1000//
#17592186044416
f028:   b2481c08orr x8, x0, #0xff00
f02c:   f2fdfff7movkx23, #0xefff, lsl #48
f030:   d344fd08lsr x8, x8, #4
f034:   38776909ldrbw9, [x8, x23]
f038:   940017d5bl  14f8c 
f03c:   5460b.eqf048   // b.none
f040:   7103fd1fcmp w8, #0xff
f044:   54000981b.nef174   // b.any
f048:   f9400014ldr x20, [x0]
if (inode->i_nlink) {
f04c:   91010280add x0, x20, #0x40
f050:   b2481c08orr x8, x0, #0xff00
f054:   d344fd08lsr x8, x8, #4
f058:   38776909ldrbw9, [x8, x23]
f05c:   940017ccbl  14f8c 
f060:   5460b.eqf06c   // b.none
f064:   7103fd1fcmp w8, #0xff
f068:   540008a1b.nef17c   // b.any
f06c:   b948ldr w8, [x0]
f070:   34000148cbz w8, f098 
f074:   940018fdbl  15468 
ret = shmem_reserve_inode(inode->i_sb);
f078:   38776909ldrbw9, [x8, x23]
f07c:   940017c4bl  14f8c 
f080:   5460b.eqf08c   // b.none
f084:   7103fd1fcmp w8, #0xff
f088:   540007e1b.nef184   // b.any
f08c:   f940ldr x0, [x0]
f090:   97fffcf6bl  e468 
if (ret)
f094:   35000660cbnzw0, f160 
dir->i_size += BOGO_DIRENT_SIZE;
f098:   910122a0add x0, x21, #0x48
f09c:   b2481c08orr x8, x0, #0xff00
f0a0:   d344fd09lsr x9, x8, #4
f0a4:   3877692aldrbw10, [x9, x23]
f0a8:   94001828bl  15148 
f0ac:   5460b.eqf0b8   // b.none
f0b0:   7103fd1fcmp w8, #0xff
f0b4:   540006c1b.nef18c   // b.any
f0b8:   38776929ldrbw9, [x9, x23]
f0bc:   94001a4abl  159e4 
f0c0:   5460b.eqf0cc   // b.none
f0c4:   7103fd1fcmp w8, #0xff
f0c8:   54000661b.nef194   // b.any
f0cc:   f909str x9, [x0]
inode->i_ctime = dir->i_ctime = dir->i_mtime = current_time(inode);
f0d0:   aa1403e0mov x0, x20
f0d4:   910182b6add x22, x21, #0x60
f0d8:   9400bl  0 
f0dc:   b2481ec9orr x9, x22, #0xff00
f0e0:   d344fd29lsr x9, x9, #4



Re: [PATCH] tmpfs: fix uninitialized return value in shmem_link

2019-02-25 Thread Darrick J. Wong
On Mon, Feb 25, 2019 at 04:07:12PM -0800, Linus Torvalds wrote:
> On Mon, Feb 25, 2019 at 4:03 PM Qian Cai  wrote:
> > >
> > > Of course, that's just gcc. I have no idea what llvm ends up doing.
> >
> > Clang 7.0:
> >
> > # clang  -O2 -S -Wall /tmp/test.c
> > /tmp/test.c:46:6: warning: variable 'ret' is used uninitialized whenever 
> > 'if'
> > condition is false [-Wsometimes-uninitialized]
> 
> Ok, good.
> 
> Do we have any clang builds in any of the zero-day robot
> infrastructure or something? Should we?
> 
> And maybe this was how Dan noticed the problem in the first place? Or
> is it just because of his eagle-eyes?

He didn't say specifically how he found it, but I would guess he was
running smatch...?

--D

>   Linus


Re: [PATCH] tmpfs: fix uninitialized return value in shmem_link

2019-02-25 Thread Linus Torvalds
On Mon, Feb 25, 2019 at 4:03 PM Qian Cai  wrote:
> >
> > Of course, that's just gcc. I have no idea what llvm ends up doing.
>
> Clang 7.0:
>
> # clang  -O2 -S -Wall /tmp/test.c
> /tmp/test.c:46:6: warning: variable 'ret' is used uninitialized whenever 'if'
> condition is false [-Wsometimes-uninitialized]

Ok, good.

Do we have any clang builds in any of the zero-day robot
infrastructure or something? Should we?

And maybe this was how Dan noticed the problem in the first place? Or
is it just because of his eagle-eyes?

  Linus


Re: [PATCH] tmpfs: fix uninitialized return value in shmem_link

2019-02-25 Thread Qian Cai



On 2/25/19 6:58 PM, Linus Torvalds wrote:
> On Mon, Feb 25, 2019 at 2:34 PM Linus Torvalds
>  wrote:
>>
>> On Mon, Feb 25, 2019 at 12:34 PM Hugh Dickins  wrote:
>>>
>>> Seems like a gcc bug? But I don't have a decent recent gcc to hand
>>> to submit a proper report, hope someone else can shed light on it.
>>
>> I don't have a _very_ recent gcc either [..]
> 
> Well, that was quick. Yup, it's considered a gcc bug.
> 
> Sadly, it's just a different version of a really old bug:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=18501
> 
> which goes back to 2004.
> 
> Which I guess means we should not expect this to be fixed in gcc any time 
> soon.
> 
> The *good* news (I guess) is that if we have other situations with
> that pattern, and that lack of warning, it really is because gcc will
> have generated code as if it was initialized (to the value that we
> tested it must have been in the one basic block where it *was*
> initialized).
> 
> So it won't leak random kernel data, and with the common error
> condition case (like in this example - checking that we didn't have an
> error) it will actually end up doing the right thing.
> 
> Entirely by mistake, and without a warniing, but still.. It could have
> been much worse. Basically at least for this pattern, "lack of
> warning" ends up meaning "it got initialized to the expected value".
> 
> Of course, that's just gcc. I have no idea what llvm ends up doing.
> 

Clang 7.0:

# clang  -O2 -S -Wall /tmp/test.c
/tmp/test.c:46:6: warning: variable 'ret' is used uninitialized whenever 'if'
condition is false [-Wsometimes-uninitialized]
if (inode->i_nlink) {
^~
/tmp/test.c:60:9: note: uninitialized use occurs here
return ret;
   ^~~
/tmp/test.c:46:2: note: remove the 'if' if its condition is always true
if (inode->i_nlink) {
^~~~
/tmp/test.c:37:9: note: initialize the variable 'ret' to silence this warning
int ret;
   ^
= 0
1 warning generated.


Re: [PATCH] tmpfs: fix uninitialized return value in shmem_link

2019-02-25 Thread Linus Torvalds
On Mon, Feb 25, 2019 at 2:34 PM Linus Torvalds
 wrote:
>
> On Mon, Feb 25, 2019 at 12:34 PM Hugh Dickins  wrote:
> >
> > Seems like a gcc bug? But I don't have a decent recent gcc to hand
> > to submit a proper report, hope someone else can shed light on it.
>
> I don't have a _very_ recent gcc either [..]

Well, that was quick. Yup, it's considered a gcc bug.

Sadly, it's just a different version of a really old bug:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=18501

which goes back to 2004.

Which I guess means we should not expect this to be fixed in gcc any time soon.

The *good* news (I guess) is that if we have other situations with
that pattern, and that lack of warning, it really is because gcc will
have generated code as if it was initialized (to the value that we
tested it must have been in the one basic block where it *was*
initialized).

So it won't leak random kernel data, and with the common error
condition case (like in this example - checking that we didn't have an
error) it will actually end up doing the right thing.

Entirely by mistake, and without a warniing, but still.. It could have
been much worse. Basically at least for this pattern, "lack of
warning" ends up meaning "it got initialized to the expected value".

Of course, that's just gcc. I have no idea what llvm ends up doing.

   Linus


Re: [PATCH] tmpfs: fix uninitialized return value in shmem_link

2019-02-25 Thread Linus Torvalds
On Mon, Feb 25, 2019 at 12:34 PM Hugh Dickins  wrote:
>
> Seems like a gcc bug? But I don't have a decent recent gcc to hand
> to submit a proper report, hope someone else can shed light on it.

I don't have a _very_ recent gcc either, but with gcc-8.2.1 the
attached test-case gives me:

   [torvalds@i7 ~]$ gcc -O2 -S -Wall test.c

with no warning, and then

   [torvalds@i7 ~]$ gcc -O2 -S -Wall -DHIDE_PROBLEM test.c
   test.c: In function ‘shmem_link’:
   test.c:60:9: warning: ‘ret’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
 return ret;
^~~

*does* show the expected warning.

So it is the presence of that

  if (ret) return ret;

that suppresses the warning.

What I *suspect* happens is

 (a) gcc sees that there is only one assignment to "ret"

 (b) in the same basic block as the assignment, there is a test
against "ret" being nonzero that goes out.

and what I think happens is that (a) causes gcc to consider that
assignment to be the defining assignment (which makes all kinds of
sense in an SSA world), and then (b) means that gcc decides that
clearly "ret" has to be zero in any case that doesn't go out due to
the if-test.

In fact, if I then look at the code generation, gcc will actually do
this (edited to be more legible):

movl(%rbx), %eax   <- load inode->i_nlink
testl   %eax, %eax
je  .L1
   ...
   ...
calld_instantiate
xorl%eax, %eax <- explicitly zero 'ret'!
.L1:
popq%rbx
popq%rbp
popq%r12
ret

so at least with my compiler, it *effectively* zeroed ret (in %rax)
anyway, and it all just _happened_ to get the right result even though
'ret' wasn't actually initialized.

Which is why it all worked just fine. And depending on how gcc works
internally, it really may not just be a random mistake of register
allocation, but really because gcc kind of _thought_ that 'ret' was
zero-initialized due to the combination of the one single assigment
and test for zero.

So it turns out that the patch to initialize to zero doesn't do
anything, probably for the same reason that gcc didn't warn about the
missing initialization. Gcc kind of added an initialization of its own
there.

I'm not entirely sure if any gcc developer would be interested in this
as a test-case, but I guess I can try to do a bugzilla.

Adding a few gcc people who have been on previous kernel gcc bugzilla
discussions, just in case they have something to add.

The gcc bugzilla is this:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89501

and I tried to make it be self-explanatory, but I wrote the bugzilla
in parallel with this email, and maybe there's some missing context
either there (or here).

 Linus
/*
 * Minimal fake declarations of "kernel" data types
 */
struct superblock;

struct inode {
	int i_nlink;
	int i_size;
	int i_ctime;
	int i_mtime;
	struct superblock *i_sb;
};

struct dentry {
	struct inode *d_inode;
};

#define d_inode(dentry) ((dentry)->d_inode)

extern int current_time(struct inode *);
extern void inc_nlink(struct inode *);
extern void ihold(struct inode *);
extern void dget(struct dentry *);
extern void d_instantiate(struct dentry *, struct inode *);
extern int shmem_reserve_inode(struct superblock *);

#define BOGO_DIRENT_SIZE 20

/*
 * The actual function where I'd have expected a warning
 * about "ret might be used uninitialized"
 */
int shmem_link(struct dentry *old_dentry, struct inode *dir,
		  struct dentry *dentry)
{
	struct inode *inode = d_inode(old_dentry);
	int ret;

	/*
	 * No ordinary (disk based) filesystem counts links as inodes;
	 * but each new link needs a new dentry, pinning lowmem, and
	 * tmpfs dentries cannot be pruned until they are unlinked.
	 * But if an O_TMPFILE file is linked into the tmpfs, the
	 * first link must skip that, to get the accounting right.
	 */
	if (inode->i_nlink) {
		ret = shmem_reserve_inode(inode->i_sb);
#ifndef HIDE_PROBLEM
		if (ret)
			return ret;
#endif
	}

	dir->i_size += BOGO_DIRENT_SIZE;
	inode->i_ctime = dir->i_ctime = dir->i_mtime = current_time(inode);
	inc_nlink(inode);
	ihold(inode);		/* New dentry reference */
	dget(dentry);		/* Extra pinning count for the created dentry */
	d_instantiate(dentry, inode);
	return ret;
}


Re: [PATCH] tmpfs: fix uninitialized return value in shmem_link

2019-02-25 Thread Hugh Dickins
On Mon, 25 Feb 2019, Linus Torvalds wrote:
> On Fri, Feb 22, 2019 at 10:35 PM Hugh Dickins  wrote:
> >
> > When we made the shmem_reserve_inode call in shmem_link conditional, we
> > forgot to update the declaration for ret so that it always has a known
> > value.  Dan Carpenter pointed out this deficiency in the original patch.
> 
> Applied.

Thanks.  And I apologize for letting that slip through: Darrick sent
the patch fragment, I dressed it up, and more or less tricked him into
taking ownership of the bug, when it's I who should have been more careful.

But I'm glad it confirmed your rc8 instinct, rather than messing final :)

> 
> Side note: how come gcc didn't warn about this? Yes, we disable that
> warning for some cases because of lots of false positives, but I
> thought the *default* setup still had it.

I thought so too, and have been puzzled by it.  If I try removing the
initialization of inode from the next function, shmem_unlink(), I do
get the expected warning for that.

> 
> Is it just that the goto ends up confusing gcc enough that it never notices?

Since the goto route did have ret properly initialized, I don't see
why it might have been confusing, but what do I know...

I thought it might be because outside the goto route, ret was used
for nothing but the return value.  But that's disproved: I tried a
very silly "inode->i_flags = ret;" just after d_instantiate(),
and still no warning when ret is uninitialized.

Seems like a gcc bug? But I don't have a decent recent gcc to hand
to submit a proper report, hope someone else can shed light on it.

Hugh


Re: [PATCH] tmpfs: fix uninitialized return value in shmem_link

2019-02-25 Thread Linus Torvalds
On Fri, Feb 22, 2019 at 10:35 PM Hugh Dickins  wrote:
>
> When we made the shmem_reserve_inode call in shmem_link conditional, we
> forgot to update the declaration for ret so that it always has a known
> value.  Dan Carpenter pointed out this deficiency in the original patch.

Applied.

Side note: how come gcc didn't warn about this? Yes, we disable that
warning for some cases because of lots of false positives, but I
thought the *default* setup still had it.

Is it just that the goto ends up confusing gcc enough that it never notices?

Linus


[PATCH] tmpfs: fix uninitialized return value in shmem_link

2019-02-22 Thread Hugh Dickins
From: "Darrick J. Wong" 

When we made the shmem_reserve_inode call in shmem_link conditional, we
forgot to update the declaration for ret so that it always has a known
value.  Dan Carpenter pointed out this deficiency in the original patch.

Fixes: 1062af920c07 ("tmpfs: fix link accounting when a tmpfile is linked in")
Reported-by: Dan Carpenter 
Signed-off-by: Darrick J. Wong 
Signed-off-by: Hugh Dickins 
---
 mm/shmem.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index 0905215fb016..2c012eee133d 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2848,7 +2848,7 @@ static int shmem_create(struct inode *dir, struct dentry 
*dentry, umode_t mode,
 static int shmem_link(struct dentry *old_dentry, struct inode *dir, struct 
dentry *dentry)
 {
struct inode *inode = d_inode(old_dentry);
-   int ret;
+   int ret = 0;
 
/*
 * No ordinary (disk based) filesystem counts links as inodes;


Re: [PATCH] tmpfs: fix uninitialized return value in shmem_link

2019-02-21 Thread Hugh Dickins
On Thu, Feb 21, 2019 at 2:21 PM Darrick J. Wong  wrote:
>
> From: Darrick J. Wong 
>
> When we made the shmem_reserve_inode call in shmem_link conditional, we
> forgot to update the declaration for ret so that it always has a known
> value.  Dan Carpenter pointed out this deficiency in the original patch.
>
> Fixes: "tmpfs: fix link accounting when a tmpfile is linked in"
> Reported-by: dan.carpen...@oracle.com
> Signed-off-by: Darrick J. Wong 

Gosh, yes indeed: thanks Dan and Darrick, I'm very sorry for not noticing that.
Acked-by: Hugh Dickins 
(and sorry if this mail is garbled: it's from gmail, I cannot use
alpine at the moment).

> ---
>  mm/shmem.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 0905215fb016..2c012eee133d 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2848,7 +2848,7 @@ static int shmem_create(struct inode *dir, struct 
> dentry *dentry, umode_t mode,
>  static int shmem_link(struct dentry *old_dentry, struct inode *dir, struct 
> dentry *dentry)
>  {
> struct inode *inode = d_inode(old_dentry);
> -   int ret;
> +   int ret = 0;
>
> /*
>  * No ordinary (disk based) filesystem counts links as inodes;


[PATCH] tmpfs: fix uninitialized return value in shmem_link

2019-02-21 Thread Darrick J. Wong
From: Darrick J. Wong 

When we made the shmem_reserve_inode call in shmem_link conditional, we
forgot to update the declaration for ret so that it always has a known
value.  Dan Carpenter pointed out this deficiency in the original patch.

Fixes: "tmpfs: fix link accounting when a tmpfile is linked in"
Reported-by: dan.carpen...@oracle.com
Signed-off-by: Darrick J. Wong 
---
 mm/shmem.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index 0905215fb016..2c012eee133d 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2848,7 +2848,7 @@ static int shmem_create(struct inode *dir, struct dentry 
*dentry, umode_t mode,
 static int shmem_link(struct dentry *old_dentry, struct inode *dir, struct 
dentry *dentry)
 {
struct inode *inode = d_inode(old_dentry);
-   int ret;
+   int ret = 0;
 
/*
 * No ordinary (disk based) filesystem counts links as inodes;