[BUG] Firmware update path: three update types report SUCCESS without flashing anything; truncated images are flashed; flash result is never checked

**Repo:** intel/xpumanager · **Version:** v2.1.0 (tag), commit `6468cec`
Package as installed: `xpu-smi 2.1.0+26.33.6468cec-1~26.04`; `xpu-smi -v` reports
`2.1.0.20250225, Build ID 8389eee7`.

## Summary

While auditing the firmware subsystem we found five independent defects that compound into
one practical conclusion: **`xpu-smi updatefw` can report success while having flashed
nothing, or while having flashed a truncated image.** Three of them are in the normal
(non-AMC) path that runs on any GPU.

⚠️ **Scope of evidence.** These come from reading the code of the released tag, not from
running firmware updates — we deliberately did not flash our cards. Line references are to
`6468cec`. Everything below is verifiable by inspection; no special hardware is needed.

## 1. Three firmware types return `ZE_RESULT_SUCCESS` without doing anything

The executor is selected purely by the `preference` field:

```cpp
// hal/core/firmware.cpp:301
fw = fwupdArray[updateFWCmds[i].preference];
```

In the command table (`hal/core/firmware.cpp:14-34`) **nine of the ten types** carry
`FWUPD_PREFERENCE_SYSMAN`; only `AMC` carries `FWUPD_PREFERENCE_AMC`.
**`FWUPD_PREFERENCE_GSC` is assigned to no type at all**, and no runtime assignment exists —
although the object is constructed:

```cpp
// hal/core/firmware.cpp:406
fwupdArray[FWUPD_PREFERENCE_GSC] = new gscupd();
```

So `gscupd` (≈760 lines of real implementation) is created and never reached.

And the SYSMAN executor is an empty class — `hal/fwupd/sysmanupd.h` in full:

```cpp
class sysmanupd : public fwupd
{
public:
sysmanupd() {}
~sysmanupd() {}
};
```

No overrides. Therefore all nine types fall through to the base-class defaults in
`hal/fwupd/fwupd.h`, and three of those defaults are no-ops:

```cpp
// hal/fwupd/fwupd.h:107
virtual ze_result_t updateGfxData(UNUSED firmwareInfo *fwInfo) { return ZE_RESULT_SUCCESS; };
// hal/fwupd/fwupd.h:119
virtual ze_result_t updateGfxCodeData(UNUSED firmwareInfo *fwInfo) { return ZE_RESULT_SUCCESS; };
// hal/fwupd/fwupd.h:122
virtual ze_result_t updateGfxPscBin(UNUSED firmwareInfo *fwInfo) { return ZE_RESULT_SUCCESS; };
```

Mapping the table to these defaults:

| firmware type | update method | effect |
|---|---|---|
| `GFX_DATA` | `updateGfxData` | **no-op, returns SUCCESS** |
| `GFX_CODE_DATA` | `updateGfxData` | **no-op, returns SUCCESS** |
| `GFX_PSCBIN` | `updateGfxPscBin` | **no-op, returns SUCCESS** |
| `GFX`, `OP_CODE`, `OP_DATA`, `FAN_TABLE`, `VR_CONFIG`, `FDO` | call `updateFW()` | work |

**Impact:** a user updating `GFX_DATA`, `GFX_CODE_DATA` or `GFX_PSCBIN` is told the update
succeeded while the device was never written. Working implementations for these operations
exist in `gscupd`, but control never reaches them.

## 2. A partially read image is flashed as if complete

```cpp
// hal/fwupd/fwupd.cpp:22-41
std::vector buffer(length);
is.read(buffer.data(), length);
return buffer; // neither gcount() nor stream state is checked
```

The only validation is emptiness:

```cpp
// hal/fwupd/fwupd.cpp:106-110
if (fwInfo->buffer.empty()) { ERR("Firmware image is empty or unreadable"); }
```

If the read is interrupted (short file, I/O error, removable medium disappearing), the
buffer is **partially filled but not empty** — the tail stays zero-initialised — and
`zesFirmwareFlash(handle, buffer.data(), buffer.size())` receives a corrupt image of the
full expected size.

**Suggested fix:** compare `is.gcount()` with `length` and check `is.good()` before
returning; optionally verify a checksum where the image format provides one.

## 3. The result of the flash itself is never examined

```cpp
// hal/fwupd/gscupd.cpp:201-220
ret = igsc_device_fw_update_ex(&fwInfo->handle, ..., flags);

return ZE_RESULT_SUCCESS;
```

There is a blank line between the assignment and the `return`. No `if (ret != IGSC_SUCCESS)`,
no translation of the code, no message. The function reports success even if the write
failed, the image was rejected, or the device dropped mid-update.

The contrast is striking: the checks *before* the call (version compatibility, image type,
hardware configuration, firmware status) are done carefully — only the outcome of the
operation itself is ignored.

**Related, same area:** in `preUpdateGfx` (`gscupd.cpp:182`) the message
`ERR("Image is not compatible with device {}", ret)` prints `ret` left over from a
*previous* call rather than the result of the version comparison.

## 4. The cross-process firmware lock can be held by two processes at once

`oal/fs_lock.h:17` states the purpose: *"RAII cross-process lock to ensure only one firmware
update runs at a time"*. The Linux implementation (`oal/lin/fs_lock.cpp:21-50` acquire,
`:55-69` release) takes `flock(LOCK_EX|LOCK_NB)` on
`/var/lock/xpum_firmware_update.lock` and **unlinks the file on release**.

`flock` is bound to the inode, not to the name:

1. process A holds the lock;
2. process B opens the same path and waits;
3. A releases and **unlinks** the file;
4. B acquires the lock on an inode that no longer has a name;
5. process C opens the path afresh — the file is gone, so a **new inode** is created — and C
acquires the lock without contention.

**B and C now both believe they hold the single lock** — precisely what the lock exists to
prevent.

Note the Windows implementation (`oal/win/fs_lock.cpp`) does not have this hole:
`CreateFileA` is opened with share mode 0, so the file cannot be opened by another process
and cannot be deleted while open. The "close and delete" idiom is safe there and unsafe on
Linux.

**Suggested fix:** do not unlink the lock file. An empty lock file costs nothing and does
not leak.

⚠️ Secondary: the fallback path `/tmp/xpum_firmware_update.lock` lives in a world-writable
directory, so an unrelated process can take the name in advance and silently block updates.

## 5. `-y, --assumeyes` is declared but never read

The flag exists in the help text and in the option structure, but no code reads it — there
is no confirmation prompt anywhere in `updatefw`. A user who passes `-y` and a user who does
not get identical behaviour: an immediate flash. Either the confirmation is missing or the
flag is.

## Why this matters together

Individually these are ordinary bugs. Together they remove every layer that would normally
catch a bad firmware update:

- the image is not validated after reading (2),
- the flash result is not checked (3),
- three types silently do nothing at all (1),
- concurrent runs are not actually prevented (4),
- and there is no confirmation step (5).

The user-visible outcome in all cases is the same: **"success"**.

## Suggested minimum fixes

1. Give `sysmanupd` real overrides, or route `GFX_DATA` / `GFX_CODE_DATA` / `GFX_PSCBIN` to
`gscupd` — and until then, return `ZE_RESULT_ERROR_UNSUPPORTED_FEATURE` from the base-class
no-ops rather than `ZE_RESULT_SUCCESS`.
2. Validate the read in `readImage` (`gcount()` + stream state).
3. Check `ret` from `igsc_device_fw_update_ex` and propagate it.
4. Stop unlinking the lock file.
5. Either honour `-y` with a real prompt, or remove the flag.

Point 1 alone would turn a silent no-op into an honest error, which is what we would have
wanted as users.

Start at hal/core/firmware.cpp and trace updatefw through hal/fwupd/fwupd.h, sysmanupd.h, gscupd.cpp, and fwupd.cpp. Then inspect oal/lin/fs_lock.cpp and the option handling for --assumeyes. Done means unsupported operations do not report success, short reads and flash failures are surfaced, locking remains exclusive, and the confirmation option has defined behavior.

Written by the indexing model from the issue text.