Re: [PATCH v2] erofs: fix incorrect symlink detection in fast symlink

[Colin Walters]

On Mon, Sep 9, 2024, at 10:18 PM, Gao Xiang wrote:

> I know you ask for an explicit check on symlink i_size, but
> I've explained the current kernel behavior:
>    - For symlink i_size < PAGE_SIZE (always >= 4096 on Linux),
>      it behaves normally for EROFS Linux implementation;
>
>    - For symlink i_size >= PAGE_SIZE, EROFS Linux
>      implementation will mark '\0' at PAGE_SIZE - 1 in
>      page_get_link() -> nd_terminate_link() so the behavior is also
>      deterministic and not harmful to the system stability and security;

Got it, OK.

> In other words, currently i_size >= PAGE_SIZE is an undefined behavior
> but Linux just truncates the link path.

I think where we had a miscommunication is that when I see "undefined behavior" I thought you were using the formal term: https://en.wikipedia.org/wiki/Undefined_behavior

The term for what you're talking about in my experience is usually "unspecified behavior" or "implementation defined behavior" which (assuming a reasonable implementor) would include silent truncation or an explicit error, but *not* walking off the end of a buffer and writing to arbitrary other kernel memory etc.

(Hmm really given the widespread use of nd_terminate_link I guess this is kind of more of a "Linux convention" than just an EROFS one, with XFS as a notable exception?)

> For this case, to be clear I'm totally fine with the limitation,
> but I need to decide whether I should make "EROFS_SYMLINK_MAXLEN"
> as 4095 or "EROFS_SYMLINK_MAXLEN" as 4096 but also accepts
> `link[4095] == '\0'`.

Mmmm...I think PATH_MAX is conventionally taken to include the NUL; yeah see
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/namei.c?id=b40c8e7a033ff2cafd33adbe50e2a516f88fa223#n123