On Mon, Aug 30, 2021 at 9:59 AM Matthew Wilcox [off-list ref] wrote:

On Mon, Aug 30, 2021 at 09:16:14AM -0700, Suren Baghdasaryan wrote:

quoted

On Mon, Aug 30, 2021 at 1:12 AM Rasmus Villemoes
[off-list ref] wrote:

quoted

On 28/08/2021 23.47, Suren Baghdasaryan wrote:

quoted

On Fri, Aug 27, 2021 at 10:52 PM Kees Cook [off-list ref] wrote:

quoted

quoted

quoted

+   case PR_SET_VMA_ANON_NAME:
+           name = strndup_user((const char __user *)arg,
+                               ANON_VMA_NAME_MAX_LEN);
+
+           if (IS_ERR(name))
+                   return PTR_ERR(name);
+
+           for (pch = name; *pch != '\0'; pch++) {
+                   if (!isprint(*pch)) {
+                           kfree(name);
+                           return -EINVAL;

I think isprint() is too weak a check.  For example, I would suggest
forbidding the following characters: ':', ']', '[', ' '.  Perhaps

Indeed. There's also the issue that the kernel's ctype actually
implements some almost-but-not-quite latin1, so (some) chars above 0x7f
would also pass isprint() - while everybody today expects utf-8, so the
ability to put almost arbitrary sequences of chars with the high bit set
could certainly confuse some parsers. IOW, don't use isprint() at all,
just explicitly check for the byte values that we and up agreeing to
allow/forbid.

quoted

quoted

quoted

isalnum() would be better?  (permit a-zA-Z0-9)  I wouldn't necessarily
be opposed to some punctuation characters, but let's avoid creating
confusion.  Do you happen to know which characters are actually in use
today?

There's some sense in refusing [, ], and :, but removing " " seems
unhelpful for reasonable descriptors. As long as weird stuff is escaped,
I think it's fine. Any parser can just extract with m|\[anon:(.*)\]$|

I see no issue in forbidding '[' and ']' but whitespace and ':' are
currently used by Android. Would forbidding or escaping '[' and ']' be
enough?

how about allowing [0x20, 0x7e] except [0x5b, 0x5d], i.e. all printable
(including space) ascii characters, except [ \ ] - the brackets as
already discussed, and backslash because then there's nobody who can get
confused about whether there's some (and then which?) escaping mechanism
in play - "\n" is simply never going to appear. Simple rules, easy to
implement, easy to explain in a man page.

Thanks for the suggestion, Rasmus. I'm all for keeping it simple.
Kees, Matthew, would that be acceptable?

Yes, I think so.  It permits all kinds of characters that might
be confusing if passed on to something else, but we can't prohibit
everything, and forbidding just these three should remove any confusion
for any parser of /proc.  Little Bobby Tables thanks you.

Thanks for all the feedback! I think I have enough change suggestions
to resping the next revision. Will send an update later today.