linux-kernel - Re: [PATCH v2 5/6] signal: sys_sigprocmask() needs retarget_shared

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 26 Apr 2011 14:43:00 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Tejun Heo <tj@...nel.org>,
	"Nikita V. Youshchenko" <nyoushchenko@...sta.com>,
	Matt Fleming <matt@...sole-pimps.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 5/6] signal: sys_sigprocmask() needs retarget_shared_pending()

> +               sigemptyset(&new_full_set);
> +               if (how == SIG_SETMASK)
> +                       new_full_set = current->blocked;
> +               new_full_set.sig[0] = new_set;

Ugh. This is just ugly.

Could we not instead turn the whole thing into a "clear these bits"
and "set these bits", and get rid of the "how" entirely for the helper
function?

IOW, we'd have

  switch (how) {
  case SIG_BLOCK:
      clear_bits = 0;
      set_bits = new_set;
      break;
  case SIG_UNBLOCK:
      clear_bits = new_set;
      set_bits = 0;
      break
  case SIG_SET:
     clear_bits = low_bits;
     set_bits = new_set;
     break;
   default:
     return -EINVAL;
  }

and notice how you now can do that helper function *WITHOUT* any
conditionals, and just make it do

    sigprocmask(&clear, &set, NULL);

which handles all cases correctly (just "andn clear" + "or set") with
no if's or switch'es.

This is why I _detest_ that idiotic "sigprocmask()" interface. That
"how" parameter is the invention of somebody who didn't understand
sets. It's stupid. There is no reason to have multiple different set
operations, when in practice all anybody ever wants is the "clear
these bits and set those other bits" - an operation that is not only
more generic than the idiotic "how", but is _faster_ too, because it
involves no conditionals.

So I realize that we cannot get away from the broken user interface,
but I do not believe that that means that our _internal_ helper
functions should use that idiotic and broken interface!

I had basically this same comment earlier when you did something
similarly mindless for another case.

So basic rule should be: if you ever pass "how" to any helper
functions, it's broken.

                                     Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/