8 ms·

stat parsing bugs

88 points by ototot 4y ago

deleted 4y ago
[deleted]
mzs 4y ago
> sudo was bitten by this back in the day (CVE-2017-1000367): > https://www.openwall.com/lists/oss-security/2017/05/30/16 https://www.openwall.com/lists/oss-security/2017/05/30/16 https://www.openwall.com/lists/oss-security/2022/12/22/5 https://www.openwall.com/lists/oss-security/2022/12/22/5
- idealmedtech 4y ago
  > This allows any sudoers user to obtain full root privileges The way most sudoers files are set up, if you're in the wheel or sudo group, you're only a "sudo -i" from a root command prompt, so I'm not sure I see why this is a vulnerability. Can anyone elaborate?
mort96 4y ago
I've been bitten by and tried to work around this as well. From what I can tell, the best you can really do is to parse by matching up parens, but someone could totally make a program with braces in its name. If I make a binary called "foo) R 10 20 30", the /proc/<pid>/stat entry will contain "1715376 (foo) R 10 20 30) 1544883 1715376 1544883...". It's terribly non-obvious how to deal with correctly.
- st_goliath 4y ago
  > It's terribly non-obvious how to deal with correctly. Like the post says: read the whole thing into memory and do a reverse search for the last ')', i.e. strrchr Once you are aware of the problem, it's obvious how to solve it, but I do agree that the hidden danger here is not immediately obvious at first.
  - zokier 4y ago
    Note that man page says that the name is truncated to 16 chars, so if for whatever reason you don't want to do unbounded length read then you can use that
    - jyxent 4y ago
      They actually allow up to 64 characters in the kernel: https://github.com/torvalds/linux/blob/8395ae05cb5a2e31d36106e8c85efa11cda849be/fs/proc/array.c#L100 https://github.com/torvalds/linux/blob/8395ae05cb5a2e31d3610... This might just be for certain kernel things though. I don't see any regular processes that aren't truncated, but I see a bunch of kernel things that have more than 16 chars on my system.
      - deleted 4y ago
        [deleted]
- deleted 4y ago
  [deleted]
ilyt 4y ago
I wish /proc|/sys would just agree on serialization format and just serialize the data into some defined format instead of having a bunch of files that all need their own parser
- stefan_ 4y ago
  Well it's too late now. But I thought the plan was for all of that stuff to move to Netlink? Not that that isn't a terrible very horrible API either.
- bradfitz 4y ago
  Advent of Proc
- jcelerier 4y ago
  we could even name the tool to query such serialized data, procctl, provided by the systemd-proc package
- st_goliath 4y ago
  While procfs has a lot of historical baggage, sysfs is rather specific about the layout and providing only a single value per file, as plain ASCII, rather than using anything complex that has to be parsed. Structure is implemented via the filesystem. In return, the kernel side API for sysfs is also a lot cleaner and allows to more-or-less expose individual variables as tuning knobs for a driver. Of course there are edge cases, and there are e.g. some binary interfaces as well (e.g. for providing direct register access, or implementing a firmware upload interface for a device). ABI compat issues aside, I think that implementing "a standardized [structured] record format" as suggested in the comments here is a rather bad idea, going into exactly the wrong direction by adding complexity rather than reducing it, which would definitely cause even more parsing related issues in the long run.
  - ilyt 4y ago
    >While procfs has a lot of historical baggage, sysfs is rather specific about the layout and providing only a single value per file, as plain ASCII, rather than using anything complex that has to be parsed. Structure is implemented via the filesystem. I'd rather have structured file than to have open 30k files (for say conntrack) Hell, just example from the article, /proc/<PID>/stat has 52 parameters. That would be 52 opens and reads with single value per file. > ABI compat issues aside, I think that implementing "a standardized [structured] record format" as suggested in the comments here is a rather bad idea, going into exactly the wrong direction by adding complexity rather than reducing it, which would definitely cause even more parsing related issues in the long run. It's literally the opposite. You have to implement it once on kernel side and once in userspace vs every special format that currently needs
esprehn 4y ago
The system level fix is to create a structured record format. That could mean quoting all the records or maybe Linux should finally adopt a standardized format like JSON.
- zokier 4y ago
  Strictly speaking it is structured; the structure is described in the man page and it is machine-parseable
  - jwilk 4y ago
    It's not described correctly. The man page says you can parse it with scanf(), which is wrong.
smasher164 4y ago
makes you wonder if it's really that valuable to have all our infrastructure built on parsing text
- raldi 4y ago
  Or to have filesystems that support names with every character but slash and NUL.
  - capitol_ 4y ago
    I agree, not supporting NUL is really a historical c-ism that we should get rid of. /s
xeeeeeeeeeeenu 4y ago
In my opinion, the fact that procfs is the only API for so many things is one of the biggest problems with Linux. BSDs have sysctl(), macOS has mach_* functions and, of course, Windows has a real API too. Plain text interfaces lead to complicated, potentially insecure code (especially in C!), they're prone to race conditions and slow. I wish it was possible to retrieve that information using real syscalls. I think it's a better approach than, for example, inventing a faster way to read procfs: https://lwn.net/Articles/813827/ https://lwn.net/Articles/813827/
- hamburglar 4y ago
  Totally agreed. Any time I’ve found myself parsing proc, I’ve felt like I was doing something foolish and unsafe in lieu of a “real” api.
- CamJN 4y ago
  MacOS’ KERN_PROCARGS2 sysctl is an exception to this, it is very unintuitive to parse and every single piece of code that tries to parse the results that I’ve found on the internet has been wrong, including those from Apple, Google, and Microsoft. I wound up making a library to do it (https://getargv.narzt.cam/ https://getargv.narzt.cam/) because apparently people need help.
  - convolvatron 4y ago
    I just ran into this and its not documented and there are very examples. I will definitely be looking at your library, thank you. it sounds fishy, but just because sysctl is a mess doesn't necessarily imply that structured kernel interfaces are a bad idea
- xxpor 4y ago
  Even if they insist on a file based interface (it is a UNIX, so fair enough), in modern times it would be nice if they used a "real" data format. Yeah, it's not like JSON parsers have never had bugs, but on average they'll be MUCH better than everyone and their mother hand rolling a C based bespoke parser. Obviously you'd need a new name to not break backwards compatibility.
bigcat12345678 4y ago
We are pixie.io ran into exact problem, we fixed that by parsing the braces, ugly but seems working https://github.com/pixie-io/pixie/blob/bd82bb48ef4da7d6b05f27fe7728dfff6687a5c6/src/common/system/proc_parser.cc#L227 https://github.com/pixie-io/pixie/blob/bd82bb48ef4da7d6b05f2...
- idealmedtech 4y ago
  That's exactly how it should be done! Also subtle but important that you find the last closing parentheses, as an attacker could just include a paren in their process name to terminate your parse early.
  - horstschneider 4y ago
    That is how psmisc does it: https://gitlab.com/psmisc/psmisc/-/blob/master/src/pstree.c#L1145 https://gitlab.com/psmisc/psmisc/-/blob/master/src/pstree.c#...
  - qwertox 4y ago
    These comments here need more visibility.
- jwilk 4y ago
  > if (std::getline(ifs, line)) { But what if comm contains newlines?
  - jwilk 4y ago
    Reported this, and a few more parsing bugs: https://github.com/pixie-io/pixie/issues/678 https://github.com/pixie-io/pixie/issues/678
cryptonector 4y ago
The process name should have been last. Now parsers have to split on space and then take the first token and the last N-2 tokens to leave behind the tokens that make up the second field, then join those with spaces to reconstruct the second field (or use the length of the first and the offset of the third fields to re-parse the second).
- tatref 4y ago
  If you do this, then you can't add new fields
  - cryptonector 4y ago
    Correct. I guess you could split on parens instead.
jbverschoor 4y ago
Why do you have to parse this kind of stuff at all? Time to let go of the everything is a stream of unorganized characters
kbrazil 4y ago
Fortunately `jc`[0] does parse `/proc/<pid>/stat` correctly. I, of course, originally implemented it the naive/incorrect way until a contributor fixed it. :) $ cat /proc/2001/stat | jc --proc {"pid":2001,"comm":"my program with\nsp","state":"S","ppid":1888,"pgrp":2001,"session":1888,"tty_nr":34816,"tpg_id":2001,"flags":4202496,"minflt":428,"cminflt":0,"majflt":0,"cmajflt":0,"utime":0,"stime":0,"cutime":0,"cstime":0,"priority":20,"nice":0,"num_threads":1,"itrealvalue":0,"starttime":75513,"vsize":115900416,"rss":297,"rsslim":18446744073709551615,"startcode":4194304,"endcode":5100612,"startstack":140737020052256,"kstkeep":140737020050904,"kstkeip":140096699233308,"signal":0,"blocked":65536,"sigignore":4,"sigcatch":65538,"wchan":18446744072034584486,"nswap":0,"cnswap":0,"exit_signal":17,"processor":0,"rt_priority":0,"policy":0,"delayacct_blkio_ticks":0,"guest_time":0,"cguest_time":0,"start_data":7200240,"end_data":7236240,"start_brk":35389440,"arg_start":140737020057179,"arg_end":140737020057223,"env_start":140737020057223,"env_end":140737020059606,"exit_code":0,"state_pretty":"Sleeping in an interruptible wait"} [0] https://kellyjonbrazil.github.io/jc/docs/parsers/proc_pid_stat https://kellyjonbrazil.github.io/jc/docs/parsers/proc_pid_st...
woodruffw 4y ago
The /proc/<pid>/* hierarchy has always been a bit of a mess to parse. /proc/<pid>/maps is similarly frustrating: there's no clear distinction between "special" maps (like the stack) and a file that might just happen to be named `[stack]`. Similarly, the handling for a mapped region on a deleted file is simply to append " (deleted)"[1]. [1]: https://github.com/woodruffw/procmaps.rs/blob/79bd474104e9b3c853e49765e6ee9945fdf833ed/src/lib.rs#L206-L225 https://github.com/woodruffw/procmaps.rs/blob/79bd474104e9b3...
inetknght 4y ago
It's almost as if there should be an API for procfs instead of having everyone write their own reader and parser...
avar 4y ago
I noticed this around a year ago when writing a /proc/paid/stat parser for git (for logging the chain of parent processes). Here's that commit, it has a comment with an overview of the kernel limits and caveats involved: https://github.com/git/git/commit/2d3491b117c6dd08e431acc3904a546c4304d276 https://github.com/git/git/commit/2d3491b117c6dd08e431acc390...
- jwilk 4y ago
  > Finally the maximum length of the "comm" name itself is 15 characters As pointed out in https://news.ycombinator.com/item?id=34098360 https://news.ycombinator.com/item?id=34098360, and contrary to the proc(5) man page, this assumption is incorrect for kernel threads.
  - avar 4y ago
    Interesting. That code will only need to the "stat" files of processes in userspace, so it's correct for its use-case. But you're right that a more general parser would need to ignore what proc(5) has to say about the limit, and parse up to a limit of 64. As far as I can tell the difference is because when you call prctl(2) with "PR_SET_NAME" it will get truncated to the "TASK_COMM_LEN" that proc(5) discusses. See this code in kernel/sys.c: https://github.com/torvalds/linux/blob/493ffd6605b2d3d4dc7008ab927dba319f36671f/kernel/sys.c#L2407-L2414 https://github.com/torvalds/linux/blob/493ffd6605b2d3d4dc700... This is the linux.git commit that changed it, before that kernel worker threads had to obey the same limit, it was first released with linux v4.18: https://github.com/torvalds/linux/commit/6b59808bfe482642287ddf3fe9d4cccb10756652 https://github.com/torvalds/linux/commit/6b59808bfe482642287...
    - jwilk 4y ago
      Your git commit message mentions PID reuse. Couldn't PID of a userspace process be reused by a kernel thread?
      - avar 4y ago
        Yes, you're right. It's been a while, I didn't think of that edge case. For the purposes of that code it's still OK. It would read a kernel thread's file, fail to find the ending ")", and stop looking. It's possible in principle that the kernel could a crafted kernel thread name that contained ")", followed by e.g. " X 12345 ". In that case I'd misinterpret that "12345" as the parent PID, and continue walking up that parent chain. But in practice the kernel doesn't have, and is exceeding unlikely to have such "comm" fields. Still, it's annoying that this was silently changed in v4.18 without a corresponding documentation update. It's easy to imagine C code written to assume its promises are true that would misbehave or segfault in the face of these longer kernel thread names. Edit: I submitted linux-man patches to clarify this point: https://lore.kernel.org/linux-man/cover-0.2-000000000-20221223T174835Z-avarab@gmail.com/ https://lore.kernel.org/linux-man/cover-0.2-000000000-202212...
YesThatTom2 4y ago
If there is exactly one field with the “may contain spaces” problem there’s a better solution: parse the line forwards for the fields up to that one, parse the line backwards for the remaining.
- jwilk 4y ago
  How is that better than looking for the last ")" character? Besides, it wouldn't work, because you don't know in advance how many fields are there.
graymatters 4y ago
Aside from bashing a paradigm one is not used to/doesn’t like/didn’t grow up with, what are the real cases where dealing with the textual output of procfs creates serious realistic performance issues? The argument about insecurity of a hand rolled C parser for that is utterly unconvincing.

88 points by ototot 4y ago

deleted 4y ago

[deleted]

mzs 4y ago

> sudo was bitten by this back in the day (CVE-2017-1000367): > https://www.openwall.com/lists/oss-security/2017/05/30/16 https://www.openwall.com/lists/oss-security/2017/05/30/16 https://www.openwall.com/lists/oss-security/2022/12/22/5 https://www.openwall.com/lists/oss-security/2022/12/22/5

idealmedtech 4y ago
> This allows any sudoers user to obtain full root privileges The way most sudoers files are set up, if you're in the wheel or sudo group, you're only a "sudo -i" from a root command prompt, so I'm not sure I see why this is a vulnerability. Can anyone elaborate?

mort96 4y ago

I've been bitten by and tried to work around this as well. From what I can tell, the best you can really do is to parse by matching up parens, but someone could totally make a program with braces in its name. If I make a binary called "foo) R 10 20 30", the /proc/<pid>/stat entry will contain "1715376 (foo) R 10 20 30) 1544883 1715376 1544883...". It's terribly non-obvious how to deal with correctly.

st_goliath 4y ago
> It's terribly non-obvious how to deal with correctly. Like the post says: read the whole thing into memory and do a reverse search for the last ')', i.e. strrchr Once you are aware of the problem, it's obvious how to solve it, but I do agree that the hidden danger here is not immediately obvious at first.
- zokier 4y ago
  Note that man page says that the name is truncated to 16 chars, so if for whatever reason you don't want to do unbounded length read then you can use that
  - jyxent 4y ago
    They actually allow up to 64 characters in the kernel: https://github.com/torvalds/linux/blob/8395ae05cb5a2e31d36106e8c85efa11cda849be/fs/proc/array.c#L100 https://github.com/torvalds/linux/blob/8395ae05cb5a2e31d3610... This might just be for certain kernel things though. I don't see any regular processes that aren't truncated, but I see a bunch of kernel things that have more than 16 chars on my system.
    - deleted 4y ago
      [deleted]
deleted 4y ago
[deleted]

ilyt 4y ago

I wish /proc|/sys would just agree on serialization format and just serialize the data into some defined format instead of having a bunch of files that all need their own parser

stefan_ 4y ago
Well it's too late now. But I thought the plan was for all of that stuff to move to Netlink? Not that that isn't a terrible very horrible API either.
bradfitz 4y ago
Advent of Proc
jcelerier 4y ago
we could even name the tool to query such serialized data, procctl, provided by the systemd-proc package
st_goliath 4y ago
While procfs has a lot of historical baggage, sysfs is rather specific about the layout and providing only a single value per file, as plain ASCII, rather than using anything complex that has to be parsed. Structure is implemented via the filesystem. In return, the kernel side API for sysfs is also a lot cleaner and allows to more-or-less expose individual variables as tuning knobs for a driver. Of course there are edge cases, and there are e.g. some binary interfaces as well (e.g. for providing direct register access, or implementing a firmware upload interface for a device). ABI compat issues aside, I think that implementing "a standardized [structured] record format" as suggested in the comments here is a rather bad idea, going into exactly the wrong direction by adding complexity rather than reducing it, which would definitely cause even more parsing related issues in the long run.
- ilyt 4y ago
  >While procfs has a lot of historical baggage, sysfs is rather specific about the layout and providing only a single value per file, as plain ASCII, rather than using anything complex that has to be parsed. Structure is implemented via the filesystem. I'd rather have structured file than to have open 30k files (for say conntrack) Hell, just example from the article, /proc/<PID>/stat has 52 parameters. That would be 52 opens and reads with single value per file. > ABI compat issues aside, I think that implementing "a standardized [structured] record format" as suggested in the comments here is a rather bad idea, going into exactly the wrong direction by adding complexity rather than reducing it, which would definitely cause even more parsing related issues in the long run. It's literally the opposite. You have to implement it once on kernel side and once in userspace vs every special format that currently needs

esprehn 4y ago

The system level fix is to create a structured record format. That could mean quoting all the records or maybe Linux should finally adopt a standardized format like JSON.

zokier 4y ago
Strictly speaking it is structured; the structure is described in the man page and it is machine-parseable
- jwilk 4y ago
  It's not described correctly. The man page says you can parse it with scanf(), which is wrong.

smasher164 4y ago

makes you wonder if it's really that valuable to have all our infrastructure built on parsing text

raldi 4y ago
Or to have filesystems that support names with every character but slash and NUL.
- capitol_ 4y ago
  I agree, not supporting NUL is really a historical c-ism that we should get rid of. /s

xeeeeeeeeeeenu 4y ago

In my opinion, the fact that procfs is the only API for so many things is one of the biggest problems with Linux. BSDs have sysctl(), macOS has mach_* functions and, of course, Windows has a real API too. Plain text interfaces lead to complicated, potentially insecure code (especially in C!), they're prone to race conditions and slow. I wish it was possible to retrieve that information using real syscalls. I think it's a better approach than, for example, inventing a faster way to read procfs: https://lwn.net/Articles/813827/ https://lwn.net/Articles/813827/

hamburglar 4y ago
Totally agreed. Any time I’ve found myself parsing proc, I’ve felt like I was doing something foolish and unsafe in lieu of a “real” api.
CamJN 4y ago
MacOS’ KERN_PROCARGS2 sysctl is an exception to this, it is very unintuitive to parse and every single piece of code that tries to parse the results that I’ve found on the internet has been wrong, including those from Apple, Google, and Microsoft. I wound up making a library to do it (https://getargv.narzt.cam/ https://getargv.narzt.cam/) because apparently people need help.
- convolvatron 4y ago
  I just ran into this and its not documented and there are very examples. I will definitely be looking at your library, thank you. it sounds fishy, but just because sysctl is a mess doesn't necessarily imply that structured kernel interfaces are a bad idea
xxpor 4y ago
Even if they insist on a file based interface (it is a UNIX, so fair enough), in modern times it would be nice if they used a "real" data format. Yeah, it's not like JSON parsers have never had bugs, but on average they'll be MUCH better than everyone and their mother hand rolling a C based bespoke parser. Obviously you'd need a new name to not break backwards compatibility.

bigcat12345678 4y ago

We are pixie.io ran into exact problem, we fixed that by parsing the braces, ugly but seems working https://github.com/pixie-io/pixie/blob/bd82bb48ef4da7d6b05f27fe7728dfff6687a5c6/src/common/system/proc_parser.cc#L227 https://github.com/pixie-io/pixie/blob/bd82bb48ef4da7d6b05f2...

idealmedtech 4y ago
That's exactly how it should be done! Also subtle but important that you find the last closing parentheses, as an attacker could just include a paren in their process name to terminate your parse early.
- horstschneider 4y ago
  That is how psmisc does it: https://gitlab.com/psmisc/psmisc/-/blob/master/src/pstree.c#L1145 https://gitlab.com/psmisc/psmisc/-/blob/master/src/pstree.c#...
- qwertox 4y ago
  These comments here need more visibility.
jwilk 4y ago
> if (std::getline(ifs, line)) { But what if comm contains newlines?
- jwilk 4y ago
  Reported this, and a few more parsing bugs: https://github.com/pixie-io/pixie/issues/678 https://github.com/pixie-io/pixie/issues/678

cryptonector 4y ago

The process name should have been last. Now parsers have to split on space and then take the first token and the last N-2 tokens to leave behind the tokens that make up the second field, then join those with spaces to reconstruct the second field (or use the length of the first and the offset of the third fields to re-parse the second).

tatref 4y ago
If you do this, then you can't add new fields
- cryptonector 4y ago
  Correct. I guess you could split on parens instead.

jbverschoor 4y ago

Why do you have to parse this kind of stuff at all? Time to let go of the everything is a stream of unorganized characters

kbrazil 4y ago

Fortunately `jc`[0] does parse `/proc/<pid>/stat` correctly. I, of course, originally implemented it the naive/incorrect way until a contributor fixed it. :) $ cat /proc/2001/stat | jc --proc {"pid":2001,"comm":"my program with\nsp","state":"S","ppid":1888,"pgrp":2001,"session":1888,"tty_nr":34816,"tpg_id":2001,"flags":4202496,"minflt":428,"cminflt":0,"majflt":0,"cmajflt":0,"utime":0,"stime":0,"cutime":0,"cstime":0,"priority":20,"nice":0,"num_threads":1,"itrealvalue":0,"starttime":75513,"vsize":115900416,"rss":297,"rsslim":18446744073709551615,"startcode":4194304,"endcode":5100612,"startstack":140737020052256,"kstkeep":140737020050904,"kstkeip":140096699233308,"signal":0,"blocked":65536,"sigignore":4,"sigcatch":65538,"wchan":18446744072034584486,"nswap":0,"cnswap":0,"exit_signal":17,"processor":0,"rt_priority":0,"policy":0,"delayacct_blkio_ticks":0,"guest_time":0,"cguest_time":0,"start_data":7200240,"end_data":7236240,"start_brk":35389440,"arg_start":140737020057179,"arg_end":140737020057223,"env_start":140737020057223,"env_end":140737020059606,"exit_code":0,"state_pretty":"Sleeping in an interruptible wait"} [0] https://kellyjonbrazil.github.io/jc/docs/parsers/proc_pid_stat https://kellyjonbrazil.github.io/jc/docs/parsers/proc_pid_st...

woodruffw 4y ago

The /proc/<pid>/* hierarchy has always been a bit of a mess to parse. /proc/<pid>/maps is similarly frustrating: there's no clear distinction between "special" maps (like the stack) and a file that might just happen to be named `[stack]`. Similarly, the handling for a mapped region on a deleted file is simply to append " (deleted)"[1]. [1]: https://github.com/woodruffw/procmaps.rs/blob/79bd474104e9b3c853e49765e6ee9945fdf833ed/src/lib.rs#L206-L225 https://github.com/woodruffw/procmaps.rs/blob/79bd474104e9b3...

inetknght 4y ago

It's almost as if there should be an API for procfs instead of having everyone write their own reader and parser...

avar 4y ago

I noticed this around a year ago when writing a /proc/paid/stat parser for git (for logging the chain of parent processes). Here's that commit, it has a comment with an overview of the kernel limits and caveats involved: https://github.com/git/git/commit/2d3491b117c6dd08e431acc3904a546c4304d276 https://github.com/git/git/commit/2d3491b117c6dd08e431acc390...

jwilk 4y ago
> Finally the maximum length of the "comm" name itself is 15 characters As pointed out in https://news.ycombinator.com/item?id=34098360 https://news.ycombinator.com/item?id=34098360, and contrary to the proc(5) man page, this assumption is incorrect for kernel threads.
- avar 4y ago
  Interesting. That code will only need to the "stat" files of processes in userspace, so it's correct for its use-case. But you're right that a more general parser would need to ignore what proc(5) has to say about the limit, and parse up to a limit of 64. As far as I can tell the difference is because when you call prctl(2) with "PR_SET_NAME" it will get truncated to the "TASK_COMM_LEN" that proc(5) discusses. See this code in kernel/sys.c: https://github.com/torvalds/linux/blob/493ffd6605b2d3d4dc7008ab927dba319f36671f/kernel/sys.c#L2407-L2414 https://github.com/torvalds/linux/blob/493ffd6605b2d3d4dc700... This is the linux.git commit that changed it, before that kernel worker threads had to obey the same limit, it was first released with linux v4.18: https://github.com/torvalds/linux/commit/6b59808bfe482642287ddf3fe9d4cccb10756652 https://github.com/torvalds/linux/commit/6b59808bfe482642287...
  - jwilk 4y ago
    Your git commit message mentions PID reuse. Couldn't PID of a userspace process be reused by a kernel thread?
    - avar 4y ago
      Yes, you're right. It's been a while, I didn't think of that edge case. For the purposes of that code it's still OK. It would read a kernel thread's file, fail to find the ending ")", and stop looking. It's possible in principle that the kernel could a crafted kernel thread name that contained ")", followed by e.g. " X 12345 ". In that case I'd misinterpret that "12345" as the parent PID, and continue walking up that parent chain. But in practice the kernel doesn't have, and is exceeding unlikely to have such "comm" fields. Still, it's annoying that this was silently changed in v4.18 without a corresponding documentation update. It's easy to imagine C code written to assume its promises are true that would misbehave or segfault in the face of these longer kernel thread names. Edit: I submitted linux-man patches to clarify this point: https://lore.kernel.org/linux-man/cover-0.2-000000000-20221223T174835Z-avarab@gmail.com/ https://lore.kernel.org/linux-man/cover-0.2-000000000-202212...

YesThatTom2 4y ago

If there is exactly one field with the “may contain spaces” problem there’s a better solution: parse the line forwards for the fields up to that one, parse the line backwards for the remaining.

jwilk 4y ago
How is that better than looking for the last ")" character? Besides, it wouldn't work, because you don't know in advance how many fields are there.

graymatters 4y ago

Aside from bashing a paradigm one is not used to/doesn’t like/didn’t grow up with, what are the real cases where dealing with the textual output of procfs creates serious realistic performance issues? The argument about insecurity of a hand rolled C parser for that is utterly unconvincing.