Bug 528581

reassigning to quota package, that syscall call looks like garbage....  apparently rpc.rquotad called

quotactl(0x580500, 0, 0, 0)

which doesn't seem to be a valid command....

Right shift that by SUBCMDSHIFT you get 0x5805 for the command, which certainly doesn't look like a valid quota command to me.  The selinux complaint is because the kernel (based on this erroneous command) attempted to either read or write to the address at special or addr, both of which are NULL.

the real bug here is that the syscall is being made with garbage.  garbage in garbage out...

Could you please provide package version of quota you are using? Additionally - are some quotas active on your system? If so, could you please briefly describe them? I'd like to have some simple reproducer. TIA.

The version is as specified in the original selinux report, quota-3.17-7.fc12.x86_64. 
I don't even have quotas enabled on the system (neither was I using nfs), its just a default rawhide Development installation plus some other development packages.

*** Bug 532342 has been marked as a duplicate of this bug. ***

This bug appears to have been reported against 'rawhide' during the Fedora 12 development cycle.
Changing version to '12'.

More information and reason for this action is here:
http://fedoraproject.org/wiki/BugZappers/HouseKeeping

This package has changed ownership in the Fedora Package Database.  Reassigning to the new owner of this component.

First argument of quotactl(2) is constructed using QCMD(cmd, type) macro that is defined as such:

#define SUBCMDMASK  0x00ff
#define SUBCMDSHIFT 8
#define QCMD(cmd, type)  (((cmd) << SUBCMDSHIFT) | ((type) & SUBCMDMASK))

Thus second byte (0x05) from failed quotactl() argument 0x580500 must originate from `cmd' and cannot be influenced by `type'. There is not much occurrences of 0x05 in sources, so I try to search in this area. Of course uninitiated values must be observed.

Other bytes from the argument can come from `type' value.

I suspect this code:

#define XQM_CMD(cmd)    ( ('X'<<8)+(cmd) )
#define Q_XGETQSTAT  XQM_CMD(0x5)   /* returns fs_quota_stat_t struct */

qcmd = QCMD(Q_XFS_GETQSTAT, 0);
[...]
if (quotactl(qcmd, h->qh_quotadev, 0, (void *)&info) < 0)
        return -1;

And really, 'X' has ASCII value 0x85, thus qcmd == 0x850500 what is our winning number.

Why magic constant 'X'? Beacuse of _X_FS. The code is from quotaio_xfs.{c,h}.

(In reply to comment #8)
s/85/58/g

The only problem is the `&info' pointer cannot be NULL because it's local on-stack structure.

Problem located:

quotasys.c:711:

void init_kernel_interface(void)
{
    struct stat st; 
    struct sigaction sig, oldsig;

    /* This signal handling is needed because old kernels send us SIGSEGV as they try to resolve the device */
    sig.sa_handler = SIG_IGN;
    sig.sa_sigaction = NULL;
    if (sigemptyset(&sig.sa_mask) < 0)
        die(2, _("Cannot create set for sigaction(): %s\n"), strerror(errno));
    sig.sa_flags = 0;
    if (sigaction(SIGSEGV, &sig, &oldsig) < 0)
        die(2, _("Cannot set signal handler: %s\n"), strerror(errno));

    kernel_formats = 0;
    if (!stat("/proc/fs/xfs/stat", &st))
        kernel_formats |= (1 << QF_XFS);
    else
→       if (!quotactl(QCMD(Q_XGETQSTAT, 0), NULL, 0, NULL) || (errno != EINVAL && errno != ENOSYS))
            kernel_formats |= (1 << QF_XFS);

Regarding comment #1: The quotactl(0x580500, 0, 0, 0) is a valid kernel command (linux-2.6.32/fs/quota/quota.c:319).

The only real problem is NULL argv[3]. quota-tools use this dummy value to detect XFS support on running kernel. However due to ambivalent zeroth page semantics on x86 kernel sometimes returns EFAULT (if the page is unmapped), sometimes 0 if the page is mapped. Therefore mmap_minaddr check has been introduced into later kernels and into SELinux policies.

I propose to use auxiliary struct fs_quota_stat as last quotactl(2) argument to make sure (1) kernel does not overwrite occasionally mapped memory region (as happened in duplicate bug #532342) and (2) too keep in-line with modern approach zeroth page is reserved as promoted by SELinux.

I will discuss this issue with upstream.

The EFAULT itself is produced earlier in quotactl(2). Kernel tries to copy second argument (name of mounted block device) into kernel space by getname() that produces EFAULT if user space source address (the second quotctl(2) argument) is NULL (if zeroth page unmapped).

We can supply "" instead of NULL. That causes ENOENT and does not trigger page fault.

The only remaining problem is I can not reproduce SELinux deny on unfixed code.

I'd like to hear from reports whether they can still reproduce the SELinux deny. I'm not able to do so on my Fedora12+testing_updates (kernel-2.6.32.12-114.fc12.x86_64, selinux-policy-3.6.32-113.fc12.noarch):

# strace -equotactl /usr/sbin/rpc.rquotad -F
quotactl(Q_XGETQSTAT|USRQUOTA, NULL, 0, NULL) = -1 EFAULT (Bad address)
^C

This adds no new line into /var/log/audit/audit.log on my system. (You can run the command as non-superuser, because quotactl check is performed before RPC bind.)

I'd like ask Dan Walsh and Eric Paris what they thing about it. Why the same syscall on the same kernel returns different values (compare duplicates of this bug report). Why SELinux does not catch access to NULL. And whether SELinux catches only write attempt or read one too (quotactl(2) exhibits both of them on NULL).

If the lowest mapping in your address space was set with MAP_GROWSDOWN you will get the SELinux denial instead of the EFAULT.  How to get into this spot with quotactl I'm not sure off the top of my head, usually it requires a long running program that has used lots of its address space and ends up putting a stack way down there.

So I could pretty easily write a test program which would cause it by calling that syscall manually with those options.....

Created attachment 411576 [details]
Prevent NULL region corruption

This is backported patch from upstream CVS tree too prevent write to NULL.

Remaining issues (read from NULL by kernel, zeroth page presence) are in discussion with upstream maintainer.

Created attachment 412858 [details]
Fix prevenemting read and write on zero page

This patch adds to the previous one NULL pointer read prevention.

The "/dev/root" device could seem silly, but the code itself does not have any meaning on modern kernels. Thus it should not have any impact, but silent SELinux. The "/dev/root" has been selected by upstream (instead of proposed "").

quota-3.17-10.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/quota-3.17-10.fc13

quota-3.17-9.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/quota-3.17-9.fc12

quota-3.17-7.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/quota-3.17-7.fc11

I have pushed the patch into all Fedoras. As nobody knows how rpc.rquotad could get mapped zero page and the NULL references has been fixed by the patch, I'll close this bug report when new releases hit stable repositories.

quota-3.17-9.fc12 has been pushed to the Fedora 12 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update quota'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/quota-3.17-9.fc12

quota-3.17-10.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

quota-3.17-9.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

quota-3.17-7.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.