Bug 1758001

+++ This bug was initially created as a clone of Bug #1757902 +++

Description of problem:
fix compat statfs64() returning EOVERFLOW for when _FILE_OFFSET_BITS=64

When compiled on 32-bit with -D_FILE_OFFSET_BITS=64, the statfs64 struct has both 32-bit and 64-bit fields.  A test of field size for the 32-bit fields erroneously also enables a check of 32-bit overflow on the 64-bit fields as well, resulting in statfs64 incorrectly returning EOVERFLOW.

Some upstream discussion of this issue occurred in 2017 and 2018 (for example, see https://patchwork.kernel.org/patch/9987759/), however no fix was applied, so the bug still exists upstream.

Version-Release number of selected component (if applicable):


How reproducible:

Test program compiled as 32-bit executable, run against a 'large' filesystem. Compiled with _FILE_OFFSET_BITS=64

Steps to Reproduce:
Diags and repro done with test 'hello world' program, which reproduces the same behavior- steps taken from Customer repro and verified:


// Simple C++ program to display "Hello World" 
  
// Header file for input output functions 
#include<iostream>  
#include<sys/statvfs.h>  
#include <errno.h>
using namespace std; 
  
// main function - 
// where the execution of program begins 
int main() 
{ 
    // prints hello world 
    cout<<"Hello World" << endl; 
      
    struct statvfs statbuf;
    int ret = statvfs("/mnt/backup",&statbuf);
    cout << "errno " << errno << endl;
    return 0; 
} 


^Repro program notice it works when compiled 64-bit but fails even when 32-bit is compiled with largefile support

[root@e1n1 mkderoy]# g++ test.cpp
[root@e1n1 mkderoy]# ./a.out
Hello World
errno 0
[root@e1n1 mkderoy]# g++ -m32 test.cpp
[root@e1n1 mkderoy]# ./a.out
Hello World
errno 75
and even with largefile support it dies
[root@e1n1 mkderoy]# g++ -m32 -D_FILE_OFFSET_BITS=64 test.cpp
[root@e1n1 mkderoy]# ./a.out
Hello World
errno 75


Actual results:

Even with 'Large File Support ' (_FILE_OFFSET_BITS=64) program returns Errno 75 (EOVERFLOW)

Expected results:

Previously, the the _FILE_OFFSET_BITS=64 flag would allow the program to run. 


Additional info:

--- Additional comment from Eric Sandeen on 2019-10-02 13:46:37 CDT ---

It would be worth stating the details of a filesystem which will cause the problem, and which field is causing the overflow for the customer.  From reading the attached cases I assume it's f_files / f_ffree, but please confirm.

--- Additional comment from Eric Sandeen on 2019-10-02 14:18:49 CDT ---

This was essentially regressed by commit 64d2ab32efe3

viro had proposed:

---
So the whole put_compat_statfs64() thing should be
static int put_compat_statfs64(struct compat_statfs64 __user *ubuf, struct kstatfs *kbuf)
{
	struct compat_statfs64 buf;

	if ((kbuf->f_bsize | kbuf->f_frsize) & 0xffffffff00000000ULL)
		return -EOVERFLOW;

	memset(&buf, 0, sizeof(struct compat_statfs64));
	buf.f_type = kbuf->f_type;
	buf.f_bsize = kbuf->f_bsize;
	buf.f_blocks = kbuf->f_blocks;
	buf.f_bfree = kbuf->f_bfree;
	buf.f_bavail = kbuf->f_bavail;
	buf.f_files = kbuf->f_files;
	buf.f_ffree = kbuf->f_ffree;
	buf.f_namelen = kbuf->f_namelen;
	buf.f_fsid.val[0] = kbuf->f_fsid.val[0];
	buf.f_fsid.val[1] = kbuf->f_fsid.val[1];
	buf.f_frsize = kbuf->f_frsize;
	buf.f_flags = kbuf->f_flags;
	if (copy_to_user(ubuf, &buf, sizeof(struct compat_statfs64)))
		return -EFAULT;
	return 0;
}
and that's it for compat_statfs64 bug
----

but that never went anywhere.  Al, did it just get lost or was there some other concern?

--- Additional comment from Chris Hudson on 2019-10-02 14:29:19 CDT ---

Thanks for the attention to this one. Our strategic partner, IBM Netezza claims that this is very impactful to their customer base and hinders certain backup functionality within their product.

Here is somewhat of an impact statement from the customer, for reference:
---
This is a major issue for us, and a major regression in RhelOS itself. All 32-bit applications that use statvfs, statfs, etc to calculate how much freespace is on a filesystem (a typical usecase) will not work on some large filesystems on rhel7 onward. We are compiling with _FILE_OFFSET_BITS=64 which should allow our application to support these filesystems as it has in the past. This is affecting our customers in a major way, as they are unable to run their backups. We expect 24*7 support to work on resolving this whether that be providing a kernel fix or an acceptable workaround for calculating free space on this filesystem as we're doing here
            int ret = statvfs(basedir.c_str(),&statbuf);
            int64 freebytes = (int64)statbuf.f_bavail * (int64)statbuf.f_bsize;
---

I have asked for a bit more history as to how things got to this point as I would expect something like this to be discovered in Netezza's QE. But it sounds like it is out in the wild and truly impacting their customers.

I have a call with this team in half an hour. Will report back with anything new that comes out of the discussion.

-Chris

--- Additional comment from Eric Sandeen on 2019-10-02 14:56:38 CDT ---

(In reply to Chris Hudson from comment #3)
> Thanks for the attention to this one.

Sure.  The kernel code in put_compat_statfs64 seems obviously broken, and needs to be fixed.  (FWIW, it's broken now in RHEL7 as well.)

I just wanted to be sure we had a clear understanding of how the customer actually hit it, and we weren't inferring anything in error.

--- Additional comment from Eric Sandeen on 2019-10-02 14:59:20 CDT ---

Just as a note, RHEL7 kernel-3.10.0-585.el7 and later (i.e. RHEL7.4+) appears to also have this flaw.

--- Additional comment from Michael DeRoy on 2019-10-02 15:17:34 CDT ---

Thank you for looking at this. Here are the details I specified in the support ticket.
example filesystem was created with a number of inodes that is above the 32-bit integer max...we did this with a 2TB partition created with these options (our customer NAS had a large number of inodes and we were able to reproduce the issue this way)
mkfs.xfs -fn ftype=1 -b size=1024 -i maxpct=99 -i size=512 /dev/nvme0n1p3

a simple c++ program reproduces this issue

// Simple C++ program to display "Hello World" 
  
// Header file for input output functions 
#include <iostream>  
#include <sys/statvfs.h>  
#include <errno.h>
using namespace std; 
  
// main function - 
// where the execution of program begins 
int main() 
{ 
	// prints hello world 
	cout<<"Hello World" << endl; 
      
	struct statvfs statbuf;
	int ret = statvfs("/badfs",&statbuf);
	cout << "errno " << errno << endl;
	return 0; 
} 

[root@e1n1 mkderoy]# g++ test.cpp
[root@e1n1 mkderoy]# ./a.out
Hello World
errno 0
[root@e1n1 mkderoy]# g++ -m32 test.cpp
[root@e1n1 mkderoy]# ./a.out
Hello World
errno 75
and even with largefile support it dies
[root@e1n1 mkderoy]# g++ -m32 -D_FILE_OFFSET_BITS=64 test.cpp
[root@e1n1 mkderoy]# ./a.out
Hello World
errno 75

Compiling with -D_FILE_OFFSET_BITS=64 works on Rhel6 and not Rhel7

--- Additional comment from Frank Sorenson on 2019-10-02 15:21:37 CDT ---

per comment 5, adding Regression

--- Additional comment from Frank Sorenson on 2019-10-02 15:23:29 CDT ---

me--  not a regression for rhel8

--- Additional comment from Eric Sandeen on 2019-10-02 15:55:45 CDT ---

If the filesystem in question is XFS, and /IF/ the customer isn't actually using nearly 2^32 files, one option may be to use xfs_growfs to reduce the maximum percentage of space which is allowable for inode allocation:

       -m     Specify a new value for  the  maximum  percentage  of  space  in  the
              filesystem  that  can  be allocated as inodes. In mkfs.xfs(8) this is
              specified with -i maxpct=nn.

This will reduce the value reported in f_files and f_ffree, and a sufficiently small percentage value will reduce this to something that doesn't overflow.

--- Additional comment from Eric Sandeen on 2019-10-02 16:19:09 CDT ---

I've sent a patch upstream:

https://lkml.org/lkml/2019/10/2/891