On Wed, 15 Oct 2014 22:24:04 +0200
Jakub Hrozek <jhrozek(a)redhat.com> wrote:
- ret = check_file(filename,
- 0, 0, S_IFSOCK|S_IRUSR|S_IWUSR, 0, NULL, true);
+ /* check_file() uses -1 as a special value that means "no
checks" */
+ check_uid = (geteuid() == 0) ? (uid_t) -1 : geteuid();
+ check_gid = (getegid() == 0) ? (gid_t) -1 : getegid();
+
+ ret = check_file(filename, check_uid, check_gid,
+ S_IFSOCK|S_IRUSR|S_IWUSR, 0, NULL, true);
It is difficult to read inlined ifs like this and in a critical
security case I would rather use some more real estate but make the
check crystal clear.
It would also be nice if the comment wouldn't state the obvious fact
that check_file() accepts -1 as a special value and rather explained
why you are using it to avoid checks.
I would do something like:
/* when running as root ignore file permission checks because .... */
check_uid = geteuid();
check_gid = getegid();
if (check_uid == 0) check_uid = -1;
if (check_gid == 0) check_gid = -1;
...
--
Simo Sorce * Red Hat, Inc * New York