Skip to content

Commit 88572c2

Browse files
committed
fix: an enquiry that reports an overflow has still answered
FILE_FS_VOLUME_INFORMATION ends in the volume's label, which is as long as the label is. The buffer here held the fixed part and one character of it -- enough for every field this reads, since the serial number precedes the label -- and the object manager still reported STATUS_BUFFER_OVERFLOW because the label did not fit. That value is 0x80000005: negative, so okw::ok said no, so the identity position was left clear. Which is a correct report of something that was not true. This implementation was saying "this node's identity is not known here", a caller was believing it, and the identity was sitting in the buffer. It surfaced two packages away, in openkal-musl's probe on Windows: "two different files have different identities" did not hold, because both had been given the zero this branch leaves behind. The conformance suite could not have said so and is right not to -- an implementation is allowed to decline the field, so the suite reports the observation as one it did not make. Room for a label is given so the ordinary case succeeds, and the overflow is accepted so the extraordinary one still answers.
1 parent c4913c5 commit 88572c2

2 files changed

Lines changed: 36 additions & 3 deletions

File tree

src/fs.cpp

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,14 +173,38 @@ int fill(void* h, kal_u32 wanted, kal_node_info* out) {
173173
// handle to something that is not on a volume --- the position is left
174174
// clear and a caller is told that this is not known, rather than being told
175175
// that two different nodes are the same.
176+
// ⚠️⚠️ THE VOLUME ENQUIRY REPORTS AN OVERFLOW AND ANSWERS ANYWAY, AND
177+
// TREATING THE OVERFLOW AS A FAILURE THREW THE ANSWER AWAY.
178+
//
179+
// FILE_FS_VOLUME_INFORMATION ends in the volume's LABEL, which is as long
180+
// as the label is. A buffer holding the fixed part and one character of it
181+
// is enough for every field this reads --- the serial number precedes the
182+
// label --- and the object manager still reports STATUS_BUFFER_OVERFLOW,
183+
// because the label did not fit. That value is 0x80000005: negative, so
184+
// `okw::ok' said no, so the position was left clear.
185+
//
186+
// ⭐ WHICH IS A CORRECT REPORT OF SOMETHING THAT WAS NOT TRUE. The
187+
// implementation was saying "this node's identity is not known here", a
188+
// caller was believing it, and the identity was sitting in the buffer. It
189+
// surfaced two packages away, in openkal-musl's probe: `two different files
190+
// have different identities' did not hold on Windows, because both had been
191+
// given the zero this branch leaves behind.
192+
//
193+
// Room for a label is given so the ordinary case SUCCEEDS, and the overflow
194+
// is accepted so the extraordinary one still answers. Both are checked
195+
// rather than one, because a label longer than this is a volume nobody
196+
// tests with and the buffer would be back to reporting an overflow.
197+
struct {
198+
okw::file_fs_volume_information info;
199+
wchar_t label_tail[128];
200+
} volume{};
176201
okw::file_internal_information index{};
177-
okw::file_fs_volume_information volume{};
178202
const long ri = okw::NtQueryInformationFile(h, &s, &index, sizeof index,
179203
okw::file_internal_information_class);
180204
const long rv = okw::NtQueryVolumeInformationFile(h, &s, &volume, sizeof volume,
181205
okw::fs_volume_information_class);
182-
if (okw::ok(ri) && okw::ok(rv)) {
183-
v.identity[0] = static_cast<kal_u64>(volume.serial_number);
206+
if (okw::ok(ri) && (okw::ok(rv) || rv == okw::status_buffer_overflow)) {
207+
v.identity[0] = static_cast<kal_u64>(volume.info.serial_number);
184208
v.identity[1] = static_cast<kal_u64>(index.index_number);
185209
v.present |= KAL_INFO_IDENTITY;
186210
}

src/win.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,15 @@ __declspec(dllimport) unsigned long __stdcall RtlNtStatusToDosError(long status)
215215

216216
inline bool ok(long status) { return status >= 0; }
217217

218+
// ⚠️ AN ENQUIRY THAT REPORTS AN OVERFLOW HAS STILL ANSWERED. STATUS_BUFFER_OVERFLOW
219+
// is a warning rather than an error: the fixed part of the structure was written
220+
// and a variable-length tail was cut. `ok' correctly says no to it --- its sign
221+
// bit is set --- so a caller that reads only fields preceding the tail names it
222+
// here. Measured: `src/fs.cpp' read a volume serial number that the object
223+
// manager had written and discarded it, and two files two packages away were
224+
// reported to have the same identity.
225+
inline constexpr long status_buffer_overflow = static_cast<long>(0x80000005ul);
226+
218227
// --- translation -------------------------------------------------------------
219228
//
220229
// The environment's error values are mapped onto the closed set the

0 commit comments

Comments
 (0)