Version 10 (2.6.24-rc1) Smack: Simplified Mandatory Access Control Kernel

Still to come:

- Final cleanup of smack_load_write and smack_cipso_write.

Bashv3 builtin "echo" behaves very strangely to -EINVAL. It sends all the buffers that causes -EINVAL again in subsequent echo invocations.

i.e.
echo "Invalid Rule" > /smack/load # -EINVAL returned echo "Valid Rule" > /smack/load

In seconod iteration, echo sends the first invalid buffer again then sends the new one. This causes a "Invalid Rule\nValid Rule" buffer sent to write().

IMHO, this is a bug in builtin echo. The external /bin/echo doesn't cause such strange behaviour.

Quoted 8 lines · Ahmed S. Darwish

Bashv3 builtin "echo" behaves very strangely to -EINVAL. It sends all the buffers that causes -EINVAL again in subsequent echo invocations.

i.e.
echo "Invalid Rule" > /smack/load # -EINVAL returned echo "Valid Rule" > /smack/load

Quoted 6 lines

In seconod iteration, echo sends the first invalid buffer again then sends the new one. This causes a "Invalid Rule\nValid Rule" buffer sent to write().

IMHO, this is a bug in builtin echo. The external /bin/echo doesn't cause such strange behaviour.

Actually, what causes problems here is something between a bug and a feature in libc's buffering. Basically the -EINVAL error causes libc to leave its data in the file-output buffer despite the file being closed and reopened. Since a standalone echo just exits that buffer is discarded, but for the bash builtin it hangs around in the buffer for a while and ends up getting prepended to the following echo statement. There's actually multiple ways to make this fail; this is just the simplest.

Quoted 3 lines

Still to come:

- Final cleanup of smack_load_write and smack_cipso_write.

Hi All,

After agreeing with Casey on the "load" input grammar yesterday, here's the final grammar and its parser (which needs more testing):

A Smack Rule in an "egrep" format is:

"^[:space:]*Subject[:space:]+Object[:space:]+[rwxaRWXA-]+[:space:]*\n"

where Subject/Object strings are in the form:

"^[^/[:space:][:cntrl:]]{1,SMK_MAXLEN}$"

Hi!

Quoted 11 lines

Quoted 3 lines

Still to come:

- Final cleanup of smack_load_write and smack_cipso_write.

Hi All,

After agreeing with Casey on the "load" input grammar yesterday, here's the final grammar and its parser (which needs more testing):

A Smack Rule in an "egrep" format is:

Quoted 5 lines

"^[:space:]*Subject[:space:]+Object[:space:]+[rwxaRWXA-]+[:space:]*\n"

where Subject/Object strings are in the form:

"^[^/[:space:][:cntrl:]]{1,SMK_MAXLEN}$"

Can we avoid string parsers in the kernel?

security/smack/smack.h static inline char *smk_of_inode(const struct inode *isp) · line 225

Quoted 5 lines

security/smack/smack.h +4 -0

+static inline int isblank(char c)
+{
+     return (c == ' ' || c == '\t');
+}

This sounds like enough for 'NAK'.

Pavel,
who still thinks smack rules should be parsed in userspace and compiled into selinux rules...

Quoted 13 lines · Pavel Machek

Hi!

Quoted 11 lines

Quoted 3 lines

Still to come:

- Final cleanup of smack_load_write and smack_cipso_write.

Hi All,

After agreeing with Casey on the "load" input grammar yesterday, here's the final grammar and its parser (which needs more testing):

A Smack Rule in an "egrep" format is:

Quoted 7 lines

Quoted 5 lines

"^[:space:]*Subject[:space:]+Object[:space:]+[rwxaRWXA-]+[:space:]*\n"

where Subject/Object strings are in the form:

"^[^/[:space:][:cntrl:]]{1,SMK_MAXLEN}$"

Can we avoid string parsers in the kernel?

I've suggested that at first, but (hoping not to misquote Al) Al viro said that the parsing is simple enough and no need exists for a user-space utility.

Quoted 8 lines

security/smack/smack.h static inline char *smk_of_inode(const struct inode *isp) · line 225

Quoted 5 lines

security/smack/smack.h +4 -0

+static inline int isblank(char c)
+{
+     return (c == ' ' || c == '\t');
+}

This sounds like enough for 'NAK'.

Would you please show the reason for the NAK so I can modify the code ?

Thank you,

Quoted 3 lines

Pavel,
who still thinks smack rules should be parsed in userspace and compiled into selinux rules...

Hi!

Quoted 11 lines

Quoted 3 lines

Still to come:

- Final cleanup of smack_load_write and smack_cipso_write.

Hi All,

After agreeing with Casey on the "load" input grammar yesterday, here's the final grammar and its parser (which needs more testing):

A Smack Rule in an "egrep" format is:

Quoted 5 lines

"^[:space:]*Subject[:space:]+Object[:space:]+[rwxaRWXA-]+[:space:]*\n"

where Subject/Object strings are in the form:

"^[^/[:space:][:cntrl:]]{1,SMK_MAXLEN}$"

Can we avoid string parsers in the kernel?

security/smack/smack.h static inline char *smk_of_inode(const struct inode *isp) · line 225

Quoted 5 lines

security/smack/smack.h +4 -0

+static inline int isblank(char c)
+{
+	return (c == ' ' || c == '\t');
+}

This sounds like enough for 'NAK'.

Pavel,
who still thinks smack rules should be parsed in userspace and compiled into selinux rules...

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Quoted 3 lines · Pavel Machek

Can we avoid string parsers in the kernel?

Ok, Could someone suggest a better idea please ?.

I thought about packing the rules in a structure and sending it over an ioctl() command. Is this applicable ?

I personally think string parsers are *much* better than the alternatives (which basically boil down to nasty binary interfaces)

Binary structures and ioctl's are *much* worse. They are totally undebuggable with generic tools (think "echo" or "strace"), and they are a total nightmare to parse across architectures and pointer sizes.

Quoted 2 lines · Linus Torvalds

I personally think string parsers are *much* better than the alternatives (which basically boil down to nasty binary interfaces)

Me too.

Quoted 3 lines

Binary structures and ioctl's are *much* worse. They are totally undebuggable with generic tools (think "echo" or "strace"), and they are a total nightmare to parse across architectures and pointer sizes.

Not only pointer sizes bugs, but also checking pointer address costs. For binary policy, we have to examine ->next pointer is valid or not. We can't blindly use address supplied from userland.

I have encountered mismatch of kernel version and AppArmor's policy parser version when I just updated only kernel. As a result, the segmentation faults rushed toward me.
From this experience, TOMOYO still uses string parser in the kernel.
If a parser doesn't consume much stack (i.e. call functions recursively), I think it is no problem.

Thanks.

You have a "\?" pattern which is defined as "1 byte character other than '/'".

The user usually doesn't know how many bytes a character in a path or file name on his system has.

Hello.

Quoted 2 lines · Adrian Bunk

You have a "\?" pattern which is defined as "1 byte character other than '/'".

Don't worry. The "\?" pattern is for temporary files with /tmp/prefixXXXXXX pattern. /tmp/prefixXXXXXX is represented as /tmp/prefix\?\?\?\?\?\? in TOMOYO Linux's syntax.

Quoted 2 lines

The user usually doesn't know how many bytes a character in a path or file name on his system has.

The "\*" pattern is for this purpose which means more than 0 byte characters other than '/'.

TOMOYO supports various patterns
http://tomoyo.sourceforge.jp/en/1.5.x/policy-reference.html#exception_policy.conf

TOMOYO Linux handles string using 7bit ASCII. In TOMOYO Linux, a byte 0x21 <= c <= 0x7E && c != 0x5C is represented as is, c == 0x5C is represented as \\,
0x01 <= c <= 0x20 || 0x7F <= c <= 0xFF is represented as \ooo style. c == 0x00 is not needed since it is used as end-of-string marker. This rule makes any string passed from/to kernel safely.

Thanks.

And \* is not a replacement for \?. It's quite common to have both ways to express "one character" and to express "at least one character", and both have their use cases and will get used if available.

But the problem is that in your code you only match one byte for \?, and this might or might not be equal to one character.

Hello.

Quoted 3 lines · Adrian Bunk

And \* is not a replacement for \?. It's quite common to have both ways to express "one character" and to express "at least one character", and both have their use cases and will get used if available.

You can use \? to represent "one character" and
\?\* to represent "at least one character".

Quoted 2 lines

But the problem is that in your code you only match one byte for \?, and this might or might not be equal to one character.

"one byte" is almost equal to "one character".
"\?" matches to one of the following types.

*  1 ASCII printable character (for 0x21-0x2E or 0x30-0x5B or 0x5D-0x7E)
*  2 ASCII printable characters \\ (for 0x5C, which means single "\")
*  4 ASCII printable characters \ooo (for 0x01-0x20 or 0x7F-0xFF, where "ooo" is octal value)

These 3 types represents one *byte*.
I want to say "\? matches to one character",
but since expression of a character depends on the value of that byte, I'm saying "\? matches to one *byte* character" instead.
Well, this sentence might be confusing, but how can I express more accurately?

Thanks.

The problem is that your code matches one byte, not one character.

More or less all userspace programs handle multi-byte UTF-8 characters just fine without bothering the user with the fact whether a character consists of one or more bytes.

And users will try to use this \? for matching one character when writing a pattern that denies access.

Hello.

Quoted 5 lines · Adrian Bunk

The problem is that your code matches one byte, not one character.

More or less all userspace programs handle multi-byte UTF-8 characters just fine without bothering the user with the fact whether a character consists of one or more bytes.

I understood what you are saying.

You are saying "a character" does not always consist of one byte, while I'm saying "a character" does always consist of one byte.

Yes, some userspace programs don't use strcmp()
since strcmp() can't handle some encodings like UTF-16. But the kernel uses strcmp()
since the VFS related functions can't handle encodings which contains '\0' in the pathname.
VFS related functions assume that '\0' is end-of-string marker.
...

Quoted 2 lines

And users will try to use this \? for matching one character when writing a pattern that denies access.

Yes, but since this string is handled by the *kernel*, I want users follow point of view of the kernel.

Thanks.

Quoted 6 lines · Ahmed S. Darwish

Quoted 3 lines · Pavel Machek

Can we avoid string parsers in the kernel?

Ok, Could someone suggest a better idea please ?.

I personally think string parsers are *much* better than the alternatives (which basically boil down to nasty binary interfaces)

Quoted 2 lines

I thought about packing the rules in a structure and sending it over an ioctl() command. Is this applicable ?

That's *MUCH* worse.

Strings are nice. They aren't that complex, and as long as it's not a performance-critical area, there are basically no downsides.

Binary structures and ioctl's are *much* worse. They are totally undebuggable with generic tools (think "echo" or "strace"), and they are a total nightmare to parse across architectures and pointer sizes.

So the rule should be: always use strings if at all possible and relevant. If the data is fundamentally binary, it shouldn't be re-coded to ascii (no real advantage), but if the data is "stringish", and there aren't big performance issues, then keep it as strings.

Quoted 2 lines · Ahmed S. Darwish

Quoted 1 line · Pavel Machek

Quoted 4 lines

Quoted 2 lines

Can we avoid string parsers in the kernel?

Ok, Could someone suggest a better idea please ?.

I personally think string parsers are *much* better than the alternatives (which basically boil down to nasty binary interfaces)
...

Linus

How do we get the information about the character encoding of the string to be parsed into the kernel?

Quoted 3 lines · Adrian Bunk

How do we get the information about the character encoding of the string to be parsed into the kernel?

I really would expect that kernel strings don't have an encoding. They're just C strings: a NUL-terminated stream of bytes.

Linus

You were the one who suggested to _parse_ strings in the kernel.

Quoted 2 lines · Adrian Bunk

You were the one who suggested to _parse_ strings in the kernel.

So?

We do that for lots of things.

What do you think a filename is? And yes, we parse it. Things like '/' and '.' and '..' have magic meaning.

You don't need to bring up idiotic things like character sets. You can see it as a byte string. You're done with it.

Linus

But if you want to match "one character" (like TOMOYO does) or want to check for printable characters except space (like Smack does) you must know whether the byte string 0xC3 0xA0 is the character à or a sequence of two characters with the second one being NBSP.

Quoted 5 lines · Adrian Bunk

But if you want to match "one character" (like TOMOYO does) or want to check for printable characters except space (like Smack does) you must know whether the byte string 0xC3 0xA0 is the character à or a sequence of two characters with the second one being NBSP.

No you don't. You just check the binary value, and decide that ' ', '\t' and '\n' are special.

Don't go get excited.

Linus

How should TOMOYO implement it's "match one character" in a pattern (used to allow or deny access in a name-based MAC)?

How should TOMOYO implement it's "match one character" in a pattern (used to allow or deny access in a name-based MAC)?

Quoted 3 lines · Adrian Bunk

How should TOMOYO implement it's "match one character" in a pattern (used to allow or deny access in a name-based MAC)?

.. I think such a design is fundamentally bogus. You don't have "characters". You have "bytes".

So you either implement "match one byte", or you go crazy. It's that simple.

Linus

Users are used to work on characters, not on bytes.

Users are used to work on characters, not on bytes.

Quoted 2 lines

So you either implement "match one byte", or you go crazy. It's that simple.

Sure, you can limit what is possible and what not.

static int smk_open_load(struct inode *inode, struct file *file) {

security/smack/smackfs.c +2 -1

-       return seq_open(file, &load_seq_ops);
+       if ((file->f_flags & O_ACCMODE) == O_RDONLY)
+               return seq_open(file, &load_seq_ops);

Quoted 4 lines · Casey Schaufler

Still to come:

- Final cleanup of smack_load_write and smack_cipso_write.

Hi All,

After agreeing with Casey on the "load" input grammar yesterday, here's the final grammar and its parser (which needs more testing):

A Smack Rule in an "egrep" format is:

"^[:space:]*Subject[:space:]+Object[:space:]+[rwxaRWXA-]+[:space:]*\n"

where Subject/Object strings are in the form:

"^[^/[:space:][:cntrl:]]{1,SMK_MAXLEN}$"

Signed-off-by Ahmed S. Darwish <[email protected]>

---

I've double-checked the code for any possible off-by-one/overflow errors.

...

Quoted 2 lines

I've double-checked the code for any possible off-by-one/overflow errors.

...

Two things caught my eye.

...

security/smack/smackfs.c static ssize_t smk_write_load(struct file *file, const char __user *buf, · line 327

Quoted 5 lines

security/smack/smackfs.c +5 -0

+             case bol:
+             case subject:
+                     if (*label_len >= SMK_MAXLEN)
+                             goto out;
+                     subjectstr[(*label_len)++] = data[i];

Why is the '>' necessary? Could it happen that you had incremented past the point of equality?

If that could not happen, then in my oppinion '>=' is very misleading when '==' is really what is needed.

...

security/smack/smackfs.c static ssize_t smk_write_load(struct file *file, const char __user *buf, · line 335

Quoted 5 lines

security/smack/smackfs.c +5 -0

+             case object:
+                     if (*prevstate == blank) {
+                             subjectstr[*label_len] = '\0';
+                             *label_len = 0;
+                     }

I wonder why it is valid to uncritically use the already incremented label_len here, without checking its value (like is done above).

It seems strangely asymmetrical. I'm not saying it's wrong, because there may be a subtle reason as to why it's not, but if that's the case then I think that subtle reason should be documented with a comment.

...

security/smack/smackfs.c static ssize_t smk_write_load(struct file *file, const char __user *buf, · line 347

Quoted 5 lines

security/smack/smackfs.c +5 -0

+             case access:
+                     if (*prevstate == blank) {
+                             objectstr[*label_len] = '\0';
+                             *label_len = 0;
+                     }

Same applies here.

/ jakob

Quoted 3 lines

A Smack Rule in an "egrep" format is:

"^[:space:]*Subject[:space:]+Object[:space:]+[rwxaRWXA-]+[:space:]*\n"

Hi!

Quoted 3 lines

Quoted 3 lines

A Smack Rule in an "egrep" format is:

"^[:space:]*Subject[:space:]+Object[:space:]+[rwxaRWXA-]+[:space:]*\n"

Perhaps you should make it space, not 'space or tab', and only allow lowercase permissions? That way, parser will be slightly simpler, and you'll still have a chance to use 'R' as 'slightly different r'.

Quoted 4 lines · Casey Schaufler

Still to come:

- Final cleanup of smack_load_write and smack_cipso_write.

Hi All,

After agreeing with Casey on the "load" input grammar yesterday, here's the final grammar and its parser (which needs more testing):

A Smack Rule in an "egrep" format is:

"^[:space:]*Subject[:space:]+Object[:space:]+[rwxaRWXA-]+[:space:]*\n"

where Subject/Object strings are in the form:

"^[^/[:space:][:cntrl:]]{1,SMK_MAXLEN}$"
...

Can you limit this to 7bit ASCII and use isascii() somewhere?

Otherwise I'd expect funny things to happen when you e.g. use isspace() on the UTF-8 encoded character à.

Quoted 4 lines · Adrian Bunk

Can you limit this to 7bit ASCII and use isascii() somewhere?

Otherwise I'd expect funny things to happen when you e.g. use isspace() on the UTF-8 encoded character à.

Actually, you don't need to. You tell them it expects UTF-8 encoded strings and be done with it. All US-ASCII characters from 0 through 127 (IE: high bit clear) are exactly the same in UTF-8, and UTF-8 special characters have the high bit set in all bytes. Therefore you just assume that anything with the high bit set is part of a word and you can handle basic UTF-8. (It doesn't work on special UTF-8 space characters like nonbreaking space and similar, but handling those is significantly more complicated).

Cheers,
Kyle Moffett

The documentations says:
"Smack labels cannot contain unprintable characters or the "/" (slash)
character."

What you propose might contain unprintable characters, and it might even be invalid UTF-8.

Quoted 6 lines · Kyle Moffett

Quoted 4 lines · Adrian Bunk

Can you limit this to 7bit ASCII and use isascii() somewhere?

Otherwise I'd expect funny things to happen when you e.g. use isspace() on the UTF-8 encoded character ??.

Quoted 8 lines

Actually, you don't need to. You tell them it expects UTF-8 encoded strings and be done with it. All US-ASCII characters from 0 through 127 (IE: high bit clear) are exactly the same in UTF-8, and UTF-8 special characters have the high bit set in all bytes. Therefore you just assume that anything with the high bit set is part of a word and you can handle basic UTF-8. (It doesn't work on special UTF-8 space characters like nonbreaking space and similar, but handling those is significantly more complicated).

The documentations says:
"Smack labels cannot contain unprintable characters or the "/" (slash)
character."

What you propose might contain unprintable characters, and it might even be invalid UTF-8.

Hi,

Quoted 16 lines · Adrian Bunk

Quoted 6 lines · Kyle Moffett

Quoted 4 lines · Adrian Bunk

Can you limit this to 7bit ASCII and use isascii() somewhere?

Otherwise I'd expect funny things to happen when you e.g. use isspace() on the UTF-8 encoded character ??.

Quoted 8 lines

Actually, you don't need to. You tell them it expects UTF-8 encoded strings and be done with it. All US-ASCII characters from 0 through 127 (IE: high bit clear) are exactly the same in UTF-8, and UTF-8 special characters have the high bit set in all bytes. Therefore you just assume that anything with the high bit set is part of a word and you can handle basic UTF-8. (It doesn't work on special UTF-8 space characters like nonbreaking space and similar, but handling those is significantly more complicated).

Quoted 6 lines

The documentations says:
"Smack labels cannot contain unprintable characters or the "/" (slash)
character."

What you propose might contain unprintable characters, and it might even be invalid UTF-8.

As far as I understand the problem now, isspace() accepts the 0xa0 character which might collide with some of UTF-8 encoded characters cause the high bit is set.

I used "if (!isspace(c) && !isgraph(c)) return -EINVAL;" to test rules' characters validity which seems not enough. I'll add !isascii(c) in the condition and ask Casey to change the documentation to be something like:

Smack labels are represented in ASCII characters, they cannot contain unprintable characters or the '/' (slash) character.

and in write():
if (!isascii(c) return -EINVAL;
if (!isspace(c) && !isgraph(c)) return -EINVAL;

This satisfy above customized labels rule, right ?

Regards,

Quoted 38 lines · Ahmed S. Darwish

Hi,

Quoted 16 lines · Adrian Bunk

Quoted 6 lines · Kyle Moffett

Quoted 4 lines · Adrian Bunk

Can you limit this to 7bit ASCII and use isascii() somewhere?

Otherwise I'd expect funny things to happen when you e.g. use isspace() on the UTF-8 encoded character ??.

Quoted 8 lines

Actually, you don't need to. You tell them it expects UTF-8 encoded strings and be done with it. All US-ASCII characters from 0 through 127 (IE: high bit clear) are exactly the same in UTF-8, and UTF-8 special characters have the high bit set in all bytes. Therefore you just assume that anything with the high bit set is part of a word and you can handle basic UTF-8. (It doesn't work on special UTF-8 space characters like nonbreaking space and similar, but handling those is significantly more complicated).

Quoted 6 lines

The documentations says:
"Smack labels cannot contain unprintable characters or the "/" (slash)
character."

What you propose might contain unprintable characters, and it might even be invalid UTF-8.

As far as I understand the problem now, isspace() accepts the 0xa0 character which might collide with some of UTF-8 encoded characters cause the high bit is set.

I used "if (!isspace(c) && !isgraph(c)) return -EINVAL;" to test rules' characters validity which seems not enough. I'll add !isascii(c) in the condition and ask Casey to change the documentation to be something like:

Smack labels are represented in ASCII characters, they cannot contain unprintable characters or the '/' (slash) character.

Quoted 5 lines

and in write():
if (!isascii(c) return -EINVAL;
if (!isspace(c) && !isgraph(c)) return -EINVAL;

This satisfy above customized labels rule, right ?

It should work for all charsets you'll usually have to handle.

It would not work if someone would e.g. give you UTF-16 encoded strings, but I don't see this happening in practice.

Quoted 2 lines · Adrian Bunk

Quoted 1 line · Ahmed S. Darwish

As far as I understand the problem now, isspace() accepts the 0xa0 character which might collide with some of UTF-8 encoded characters cause the high bit is set.

I admit I'm not experienced in such encoding stuff, but shouldn't the ASCII and the ASCII-compatible UTF-8 encodings be enough for the labels?

Quoted 1 line

It would not work if someone would e.g. give you UTF-16 encoded strings, but I don't see this happening in practice.

Won't this complicate the code too much ?

Quoted 12 lines · Ahmed S. Darwish

Quoted 4 lines · Adrian Bunk

Quoted 3 lines · Ahmed S. Darwish

As far as I understand the problem now, isspace() accepts the 0xa0 character which might collide with some of UTF-8 encoded characters cause the high bit is set.

I admit I'm not experienced in such encoding stuff, but shouldn't the ASCII and the ASCII-compatible UTF-8 encodings be enough for the labels?

Quoted 2 lines

It would not work if someone would e.g. give you UTF-16 encoded strings, but I don't see this happening in practice.

Quoted 1 line

Won't this complicate the code too much ?

Well the VFS (for example) certainly doesn't support any encodings other than various extended-ASCII forms (which includes UTF-8). Something like UTF-16 has extra null characters in-between every normal character, and as such would fail completely if passed to the VFS.

Personally I think that isspace() accepting character 0xA0 is a bug, as there are several variants of extended ASCII only one of which has that character as a space. Others have it as á (accented A), etc.

In addition
the "canonical" internal text format of the kernel is UTF-8 as that encoding can represent any character in any other encoding and it is backwards-compatible with traditional ASCII.

Cheers,
Kyle Moffett-

Quoted 13 lines · Kyle Moffett

Quoted 12 lines · Ahmed S. Darwish

Quoted 4 lines · Adrian Bunk

Quoted 3 lines · Ahmed S. Darwish

As far as I understand the problem now, isspace() accepts the 0xa0 character which might collide with some of UTF-8 encoded characters cause the high bit is set.

I admit I'm not experienced in such encoding stuff, but shouldn't the ASCII and the ASCII-compatible UTF-8 encodings be enough for the labels?

Quoted 2 lines

It would not work if someone would e.g. give you UTF-16 encoded strings, but I don't see this happening in practice.

Quoted 6 lines

Quoted 1 line

Won't this complicate the code too much ?

Well the VFS (for example) certainly doesn't support any encodings other than various extended-ASCII forms (which includes UTF-8). Something like UTF-16 has extra null characters in-between every normal character, and as such would fail completely if passed to the VFS.

Good point.

Quoted 3 lines

Personally I think that isspace() accepting character 0xA0 is a bug, as there are several variants of extended ASCII only one of which has that character as a space. Others have it as ?? (accented A), etc.

But even then Smack would still have a similar problem with isgraph().

Quoted 7 lines

In addition
the "canonical" internal text format of the kernel is UTF-8 as that encoding can represent any character in any other encoding and it is backwards-compatible with traditional ASCII.

Cheers,
Kyle Moffett-

cu
Adrian

Quoted 27 lines · Adrian Bunk

Quoted 13 lines · Kyle Moffett

Quoted 12 lines · Ahmed S. Darwish

Quoted 4 lines · Adrian Bunk

Quoted 3 lines · Ahmed S. Darwish

As far as I understand the problem now, isspace() accepts the 0xa0 character which might collide with some of UTF-8 encoded characters cause the high bit is set.

I admit I'm not experienced in such encoding stuff, but shouldn't the ASCII and the ASCII-compatible UTF-8 encodings be enough for the labels?

Quoted 2 lines

It would not work if someone would e.g. give you UTF-16 encoded strings, but I don't see this happening in practice.

Quoted 6 lines

Quoted 1 line

Won't this complicate the code too much ?

Well the VFS (for example) certainly doesn't support any encodings other than various extended-ASCII forms (which includes UTF-8). Something like UTF-16 has extra null characters in-between every normal character, and as such would fail completely if passed to the VFS.

Good point.

Quoted 3 lines

Personally I think that isspace() accepting character 0xA0 is a bug, as there are several variants of extended ASCII only one of which has that character as a space. Others have it as ?? (accented A), etc.

Quoted 2 lines

But even then Smack would still have a similar problem with isgraph().

Great, To summarize the discussion. Will there be a problem in accepting ASCII and the UTF-8 ASCII _subset_ _only_ and return -EINVAL for all other cases/ecnodings ?.

i.e. The fragment I sent in a previous message:

/* Filter UTF-8 non-ascii compatible bytes (> 0x7F) */ if (!isascii(c)) return -EINVAL;
/* Filter unwanted ascii chars */
if (!isspace(c) && !isgraph(c)) return -EINVAL;
...

Quoted 4 lines · Ahmed S. Darwish

Great, To summarize the discussion. Will there be a problem in accepting ASCII and the UTF-8 ASCII _subset_ _only_ and return -EINVAL for all other cases/ecnodings ?.

The UTF-8 ASCII subset is the complete ASCII.

Quoted 2 lines

i.e. The fragment I sent in a previous message:

Quoted 5 lines

/* Filter UTF-8 non-ascii compatible bytes (> 0x7F) */ if (!isascii(c)) return -EINVAL;
/* Filter unwanted ascii chars */
if (!isspace(c) && !isgraph(c)) return -EINVAL;
...

As I already said, this should work fine.

Personally I think that isspace() accepting character 0xA0 is a bug

Quoted 1 line · Kyle Moffett

Personally I think that isspace() accepting character 0xA0 is a bug

I think I agree with you. As far as the kernel is concerned, "isspace()" should just accept the obvious spaces (hardspace, tab, newline), and *perhaps* the VT/FF kind of things.

You should realize that the kernel <ctype.h> thing is *ancient*. It's basically there from v0.01, and while the really original one (I just checked) had all the non-ascii characters not trigger anything, it was converted to be latin1 in the 2.1.x timeframe.

That's a *loong* time ago. Way before UTF-8 and other things were really common.

So we should probably just make all the upper 128 bytes go back to "don't trigger anything in ctype.h" - they'd not be spaces, but they'd not be control characters or anything else either.