!ioctl: serialize nested ioctls outside of kernel

[julianuziemblo]

Description

Motivation and Context

Depends-On: phoenix-rtos-kernel:julianuziemblo/implement-siocethtool
Depends-On: phoenix-rtos-lwip:julianuziemblo/RTOS-1205
Depends-On: phoenix-rtos-tests:julianuziemblo/RTOS-1205

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality):
  Serialization and deserialization is
• Breaking change (fix or feature that would cause existing functionality to change):
  Changed signature of ioctl syscall proxy function to:
  int sys_ioctl(int fildes, unsigned long request, void *val, size_t size)
  This was done to pass the size of the size of the whole structure to the kernel without affecting the request variable, so we can still match on it in the device server handling the ioctl. This eliminates the need to handle special cases in the kernel: userspace serializes the ioctl, and device server unpacks it.
• Chore (refactoring, style fixes, git/CI config, submodule management, no code logic changes)

How Has This Been Tested?

• Already covered by automatic testing.
• New test added: ioctl: add nested ioctl tests phoenix-rtos-tests#447
• Tested by hand on: (list targets here).

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing linter checks and tests passed.
• My changes generate no new compilation warnings for any of the targets.

Special treatment

• This PR needs additional PRs to work:
  • include/lwip/sockets: adjust to Phoenix-RTOS ioctl interface lwip#18
  • port/sockets: adjust to special ioctl changes phoenix-rtos-lwip#134
  • !ioctl: serialize nested ioctls outside of kernel phoenix-rtos-kernel#656
• I will merge this PR by myself when appropriate.

[gemini-code-assist]

Summary of Changes

Hello @julianuziemblo, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly refactors the ioctl system call mechanism by shifting the responsibility of handling complex argument serialization and deserialization from the kernel to userspace. This change aims to streamline the kernel's ioctl implementation, making it more generic and easier to maintain, while providing a robust and extensible framework for managing diverse ioctl argument types in the userspace library. The modification introduces a new helper module to manage this process, ensuring that structures with embedded pointers are correctly packed and unpacked before and after kernel calls.

Highlights

• Centralized IOCTL Argument Handling: The core change involves moving the serialization and deserialization of complex ioctl arguments (especially those containing pointers) from kernel-side special-case handling to a new userspace helper library. This simplifies kernel logic and centralizes argument packing/unpacking.
• Updated sys_ioctl Signature: The sys_ioctl proxy function's signature has been modified to int sys_ioctl(int fildes, unsigned long request, void *val, size_t size). This new size parameter allows passing the total size of the serialized structure to the kernel without altering the request variable, which is crucial for device server matching.
• New IOCTL Helper Library: A new ioctl-helper.c and ioctl-helper.h pair has been introduced in unistd/ to manage the serialization, deserialization, and memory management for ioctl arguments. This library handles specific requests like SIOCGIFCONF, SIOCADDRT, and SIOCDELRT.
• Refactored ioctl Wrapper: The userspace ioctl wrapper function in unistd/file.c now utilizes the new ioctl_serialize, sys_ioctl, and ioctl_deserialize functions to prepare and process ioctl arguments, ensuring proper handling of complex data structures.
• IOCTL Macro Adjustments: Several ioctl-related macros in include/sys/ioctl.h have been updated, including IOCPARM_MASK, IOC_VOID, and IOC_DIRMASK, and a new convenience macro IOC_GET_PTR_FIELD has been added.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request refactors the handling of special-case ioctl commands by moving serialization logic from kernel-space to user-space. This is a solid architectural improvement that simplifies the kernel's role and centralizes complex data handling within libphoenix. The introduction of ioctl-helper.c provides a scalable way to manage ioctl commands with complex data structures containing pointers. The changes to the ioctl function signature and implementation are well-contained and improve correctness, particularly in handling variadic arguments. I have one high-severity security-related comment regarding a potential buffer over-read.

[github-actions]

Unit Test Results

9 553 tests   8 961 ✅  52m 51s ⏱️
  591 suites    592 💤
    1 files        0 ❌

Results for commit d27bdb3.

♻️ This comment has been updated with latest results.