Claude review: drm/syncobj: Enforce strict checking of timeline syncobj struct

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Approach:** The patch makes two logical changes in each of the two ioctl handlers (`handle_to_fd` and `fd_to_handle`):

1. **Reject `point` without `TIMELINE` flag** — Previously, if userspace passed a non-zero `args->point` without the `TIMELINE` flag, the `point` local variable was simply left at 0 and the non-zero `args->point` was silently ignored. Now it returns `-EINVAL`. This is the fix for the described userspace bug.

2. **Reject `TIMELINE` flag without sync file flag** — Previously, after the sync file export/import path, the code checked `if (args->point)` to reject timeline usage on the non-sync-file path. Now it checks `if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE)` instead. This is more correct since after change #1, `point` could legitimately be 0 with the `TIMELINE` flag set.

**Positive aspects:**
- The commit message clearly explains the motivation (userspace bug with uninitialized `args->point`).
- Removing the `point` local variable and using `args->point` directly simplifies the code.
- The `Fixes:` tag is appropriate.

**Concern — ABI break for existing userspace:**

The new check at line 870/901:
```c
if (!(args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE) && args->point)
    return -EINVAL;
```

This will **break existing userspace** that has the exact bug described in the commit message — passing garbage in `args->point` without the `TIMELINE` flag. The old code silently ignored this; the new code fails with `-EINVAL`. While the intent is to force userspace to fix their bugs, this is technically a UAPI regression. Any deployed userspace with the uninitialized `args->point` bug that previously worked will now fail.

The kernel's usual stance is "don't break userspace." The commit message acknowledges this was "discovered when userspace increased the size of the ioctl to include args->point, but never cleared args->point," which suggests a real application is affected. The question is: should the kernel reject this (forcing a userspace fix) or continue to be permissive (as it was before)?

Given that the `TIMELINE` extensions are relatively new and the affected userspace is presumably being fixed concurrently, this is probably acceptable, but it would be worth confirming that the affected userspace fix is coordinated. A `Cc: stable` tag might also be appropriate if backporting is desired to prevent the buggy userspace pattern from becoming more entrenched.

**Minor nit — TIMELINE + EXPORT_SYNC_FILE with point=0:**

With this patch, `TIMELINE` flag + `EXPORT_SYNC_FILE` flag + `point=0` is now accepted and passed through to `drm_syncobj_export_sync_file()`. Previously this also worked (point would be set to 0 from `args->point`). This is fine — timeline point 0 typically means "current fence" — but worth noting that the semantics are preserved.

**Overall:** The patch is correct and well-motivated. The only substantive concern is the potential for breaking existing (buggy) userspace, which the commit message should address more explicitly — perhaps noting that the broken userspace in question is being fixed in tandem, or adding a comment about the deliberate ABI tightening.

---
Generated by Claude Code Patch Reviewer