Fix the incorrect logical operator in AUDITPIPE_SET_QLIMIT ioctl
ClosedPublic
Actions

Authored by aniketp on Jul 23 2018, 4:24 PM.

Details

Reviewers

asomers
cem

Summary

The logical operator which verifies that the desired limit of auditpipe queue length to be set is
between QLIMIT_MIN and QLIMIT_MAX is wrong.

case AUDITPIPE_SET_QLIMIT:
	/* Lockless integer write. */
	if (*(u_int *)data >= AUDIT_PIPE_QLIMIT_MIN ||
            *(u_int *)data <= AUDIT_PIPE_QLIMIT_MAX) {

should be

case AUDITPIPE_SET_QLIMIT:
	/* Lockless integer write. */
	if (*(u_int *)data >= AUDIT_PIPE_QLIMIT_MIN &&
            *(u_int *)data <= AUDIT_PIPE_QLIMIT_MAX) {

Bug Report: PR: 229983

Test Plan

Steps to reproduce the bug: (On 12-CURRENT)

#include <stdio.h>
#include <fcntl.h>
#include <unistd.h>
#include <sys/ioctl.h>
#include <security/audit/audit_ioctl.h>

void main() {
	int fd = open("/dev/auditpipe", O_RDWR);
        if (fd < 0)
               perror("auditpipe");

	int qlimit_min;	
	ioctl(fd, AUDITPIPE_GET_QLIMIT_MIN, &qlimit_min);

        qlimit_min -= 5;     \* Not allowed since it is less than QLIMIT_MIN *\
	
        ioctl(fd, AUDITPIPE_SET_QLIMIT, &qlimit_min);
        perror("set qlimit");
	close(fd);
}

Output: "set qlimit: No error: 0"

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 18270
Build 17995: arc lint + arc unit

Event Timeline

aniketp created this revision.Jul 23 2018, 4:24 PM

Harbormaster completed remote builds in B18270: Diff 45731.Jul 23 2018, 4:24 PM

I see avg already committed it.

This revision is now accepted and ready to land.Jul 23 2018, 5:05 PM

Fixed by r336641

Revision Contents
Changeset List

Path

Size

sys/

security/

audit/

audit_pipe.c

2 lines

Diff 45731

View Options

sys/security/audit/audit_pipe.c

Show First 20 Lines • Show All 81 Lines • ▼ Show 20 Lines	FBSD_1.0 {
fchdir;		fchdir;
fchflags;		fchflags;
fchmod;		fchmod;
fchown;		fchown;
fcntl;		fcntl;
fhopen;		fhopen;
flock;		flock;
fork;		fork;
fpathconf;		fpathconf;
		kibUnsubmitted Done Inline Actions Add context to the diff. Even without the context, I am sure that you put the symbols into the wrong namespace. kib: Add context to the diff. Even without the context, I am sure that you put the symbols into the…
		jack_gandi.netAuthorUnsubmitted Done Inline Actions Should I add a FBSD_1.6 section then? jack_gandi.net: Should I add a FBSD_1.6 section then?
		jack_gandi.netAuthorUnsubmitted Done Inline Actions nevermind, I just realized https://wiki.freebsd.org/AddingSyscalls says to put them in the most recent namespace (FBSD_1.5) jack_gandi.net: nevermind, I just realized https://wiki.freebsd.org/AddingSyscalls says to put them in the most…
fsync;		fsync;
futimes;		futimes;
getaudit;		getaudit;
getaudit_addr;		getaudit_addr;
getauid;		getauid;
getcontext;		getcontext;
getdtablesize;		getdtablesize;
getegid;		getegid;
▲ Show 20 Lines • Show All 285 Lines • ▼ Show 20 Lines	FBSD_1.5 {
fdatasync;		fdatasync;
fhstat;		fhstat;
fhstatfs;		fhstatfs;
fstat;		fstat;
fstatat;		fstatat;
fstatfs;		fstatfs;
getdents;		getdents;
getdirentries;		getdirentries;
getfsstat;		getfsstat;
		kibUnsubmitted Not Done Inline Actions No, FBSD_1.5 is wrong again. New 13-CURRENT symbols must go into FBSD_1.6. kib: No, FBSD_1.5 is wrong again. New 13-CURRENT symbols must go into FBSD_1.6.
getrandom;		getrandom;
kevent;		kevent;
lstat;		lstat;
mknod;		mknod;
mknodat;		mknodat;
stat;		stat;
statfs;		statfs;
cpuset_getdomain;		cpuset_getdomain;
cpuset_setdomain;		cpuset_setdomain;
};		};

		FBSD_1.6 {
		fhlink;
		fhlinkat;
		fhreadlink;
		getfhat;
		};

FBSDprivate_1.0 {		FBSDprivate_1.0 {
___acl_aclcheck_fd;		___acl_aclcheck_fd;
__sys___acl_aclcheck_fd;		__sys___acl_aclcheck_fd;
___acl_aclcheck_file;		___acl_aclcheck_file;
__sys___acl_aclcheck_file;		__sys___acl_aclcheck_file;
___acl_aclcheck_link;		___acl_aclcheck_link;
__sys___acl_aclcheck_link;		__sys___acl_aclcheck_link;
___acl_delete_fd;		___acl_delete_fd;
▲ Show 20 Lines • Show All 153 Lines • ▼ Show 20 Lines	FBSDprivate_1.0 {
__sys_fchown;		__sys_fchown;
_fcntl;		_fcntl;
__sys_fcntl;		__sys_fcntl;
__fcntl_compat;		__fcntl_compat;
_fhopen;		_fhopen;
__sys_fhopen;		__sys_fhopen;
_fhstat;		_fhstat;
__sys_fhstat;		__sys_fhstat;
_fhstatfs;		_fhstatfs;
		brooksUnsubmitted Done Inline Actions I believe it's unnecessary to add these as there are no potential consumers of these symbols. brooks: I believe it's unnecessary to add these as there are no potential consumers of these symbols.
__sys_fhstatfs;		__sys_fhstatfs;
_flock;		_flock;
__sys_flock;		__sys_flock;
_fork;		_fork;
__sys_fork;		__sys_fork;
_fpathconf;		_fpathconf;
__sys_fpathconf;		__sys_fpathconf;
_fstat;		_fstat;
▲ Show 20 Lines • Show All 443 Lines • Show Last 20 Lines

Fix the incorrect logical operator in AUDITPIPE_SET_QLIMIT ioctlClosedPublicActions