Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure NFSv4 data is parsed correctly #14120

Closed

Conversation

Leseratte10
Copy link
Contributor

@Leseratte10 Leseratte10 commented Aug 1, 2024

I noticed on my TrueNAS Scale 24.04.2 install that A) the display of active NFSv4 sessions isn't working and B) I get the following errors in the log:

Error log
[2024/07/30 19:07:33] (WARNING) application.call_method():234 - Exception while calling nfs.get_nfs4_clients(*[])
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/middlewared/main.py", line 198, in call_method
    result = await self.middleware.call_with_audit(message['method'], serviceobj, methodobj, params, self)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/middlewared/main.py", line 1466, in call_with_audit
    result = await self._call(method, serviceobj, methodobj, params, app=app,
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/middlewared/main.py", line 1428, in _call
    return await self.run_in_executor(prepared_call.executor, methodobj, *prepared_call.args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/middlewared/main.py", line 1321, in run_in_executor
    return await loop.run_in_executor(pool, functools.partial(method, *args, **kwargs))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/middlewared/schema/processor.py", line 191, in nf
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/middlewared/schema/processor.py", line 53, in nf
    res = f(*args, **kwargs)
          ^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/middlewared/plugins/nfs_/status.py", line 155, in get_nfs4_clients
    "info": self.get_nfs4_client_info(client),
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/middlewared/plugins/nfs_/status.py", line 54, in get_nfs4_client_info
    info = yaml.safe_load(f.read())
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/yaml/__init__.py", line 125, in safe_load
    return load(stream, SafeLoader)
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/yaml/__init__.py", line 81, in load
    return loader.get_single_data()
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/yaml/constructor.py", line 49, in get_single_data
    node = self.get_single_node()
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/yaml/composer.py", line 36, in get_single_node
    document = self.compose_document()
               ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/yaml/composer.py", line 55, in compose_document
    node = self.compose_node(None, None)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/yaml/composer.py", line 84, in compose_node
    node = self.compose_mapping_node(anchor)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/yaml/composer.py", line 127, in compose_mapping_node
    while not self.check_event(MappingEndEvent):
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/yaml/parser.py", line 98, in check_event
    self.current_event = self.state()
                         ^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/yaml/parser.py", line 438, in parse_block_mapping_key
    raise ParserError("while parsing a block mapping", self.marks[-1],
yaml.parser.ParserError: while parsing a block mapping
  in "<unicode string>", line 1, column 1:
    clientid: 0xf911111111111111
    ^
expected <block end>, but found '<scalar>'
  in "<unicode string>", line 11, column 56:
     ... :1234:1234:0:1234:56ff:fe78:90ab]:0

This is because the code currently assumes that that data coming from the Linux kernel is valid YAML, but there's one parameter in that data that's not. Here's an example from my machine:

clientid: 0xf911111111111111
address: "[fd00:1234:1234:0:1234:56ff:fe78:90ab]:706"
status: confirmed
seconds from last renew: 48
name: "Linux NFSv4.2 my-machine"
minor version: 2
Implementation domain: "kernel.org"
Implementation name: "Linux 5.15.0-113-generic #123~20.04.1-Ubuntu SMP Wed Jun 12 17:33:13 UTC 2024 x86_64"
Implementation time: [0, 0]
callback state: UP
callback address: [fd00:1234:1234:0:1234:56ff:fe78:90ab]:0

As you can see, the callback address isn't quoted despite having multiple colons in the data, so it's not valid YAML.
This causes middlewared to fail to parse that as YAML, which results in the error "Can not retrieve response" when trying to list NFS4 sessions in the WebUI.

A good, long-term solution would probably be to no longer parse this data as YAML and write a custom parser instead.

For the meantime, because it was easier, I added a check for "callback address" and then just add quotation marks myself before parsing it as YAML. Not sure if that's an acceptable solution, but it's a heck of a lot better than a broken page with an error message as soon as a client is connected using NFSv4, and I was able to reproduce the issue (and confirm the fix works) even on a fully new stock install of TrueNAS Scale).

@pcbsd-commit-bot
Copy link

Can one of the admins verify this patch?

@Leseratte10 Leseratte10 force-pushed the fix-nfs4-client-ip-display branch 2 times, most recently from eae6a83 to c936339 Compare August 1, 2024 15:06
@Leseratte10 Leseratte10 force-pushed the fix-nfs4-client-ip-display branch from c936339 to 57eac3f Compare August 1, 2024 15:06
@anodos325
Copy link
Contributor

We're going to fix sysfs output so that it's valid YAML.

@anodos325 anodos325 closed this Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants