That's pretty weird.

registry_test.go:569: value type 2097152 of DevLoader of LOCAL_MACHINE\SYSTEM\ControlSet001\Control\Class\{4d36e96c-e325-11ce-bfc1-08002be10318}\Configuration\Reset\Driver failed: <nil> implies that:

t.Fatalf("value type %d of %s of %s failed: %v", valtype, name, kname, err)

-->

_, valtype, err := k.GetValue(name, nil)

-->

err = syscall.RegQueryValueEx(syscall.Handle(k), pname, nil, &valtype, pbuf, &l)

I don't see anything in docs about RegQueryValueEx returning 0x200000.

Actually, no, it's not weird. That's just the type of those entries in the registry. New wild undocumented Windowsism? Investigating...

Looks like that's coming from C:\Windows\INF\c_media.inf:

[ClassInstall32.Configuration]
AddReg = ClassConfiguration_AddReg

[ClassConfiguration_AddReg]
...
HKR,Reset\Driver,DevLoader,%FLG_ADDREG_DELVAL%
...

[Strings]
...
FLG_ADDREG_DELVAL   = 0x00000004

That looks in theory fine, but I wonder if the INF loader isn't used to pulling integer flags out of the [Strings] section like that.

Either way, looks like a Windows bug.

Onto the remaining integer size mysteries.

It appears that indeed that registry key has the wrong sized value:

Looks like a Windows bug.

So the Go test is wrong, I guess.

@bradfitz What's the deal with the long tests? Are they something that nobody ever runs, hence you discovering this only now?

@zx2c4, yup... https://golang.org/cl/202479 "dashboard: add windows and 32-bit longtest builders" for #26529

Change https://golang.org/cl/202678 mentions this issue: internal/syscall/windows/registry: fix strict assumptions in TestWalkFullRegistry

@bradfitz You wanted more detail over on the CL. Here's the root cause for the integer size bug:

They're writing an integer of size 8 as type REG_DWORD, which REG_DWORD requires an integer of size 4. The key is called "lParam", which means I bet this was originally of type LPARAM which is of type LONG_PTR which is the size of a pointer. So probably the original C code was something like:

RegSetValueExW(hKey, L"lParam", 0, REG_DWORD, (const BYTE *)&whatever->lParam, sizeof(whatever->lParam));

This worked fine in the 32-bit Windows days, and then broke when they recompiled that binary for 64-bit.

I only know how to report security bugs to MSRC, but not other bugs to whatever channels Microsoft has for those. If somebody wants to direct a Microsoft person toward this thread, maybe this can get fixed.

@zx2c4, nice, thanks.

Maybe we can get away with a whitelist of known registry entries to skip, then?

I suspect the point of that test was just to exercise all the code paths by throwing a lot of mixed data at it.

@bradfitz Done. Not pretty, but it seems to be working, at least on my machine.

Change https://golang.org/cl/202897 mentions this issue: internal/syscall/windows/registry: blacklist certain registry keys in TestWalkFullRegistry

Here's the root cause for the integer size bug:

They're writing an integer of size 8 as type REG_DWORD, which REG_DWORD requires an integer of size 4. The key is called "lParam", which means I bet this was originally of type LPARAM which is of type LONG_PTR which is the size of a pointer. So probably the original C code was something like:

RegSetValueExW(hKey, L"lParam", 0, REG_DWORD, (const BYTE *)&whatever->lParam, sizeof(whatever->lParam));

This worked fine in the 32-bit Windows days, and then broke when they recompiled that binary for 64-bit.

I only know how to report security bugs to MSRC, but not other bugs to whatever channels Microsoft has for those. If somebody wants to direct a Microsoft person toward this thread, maybe this can get fixed.

@jstarks maybe you could report this bug at the right channel?

Thank you.

Alex

@jstarks maybe you could report this bug at the right channel?

Also the INF one too, above.

Thanks for pointing this out; I can report these internally. But I think you'll have to keep your ignore list around, since it seems unlikely that we would backport fixes for these bugs.

Thanks for pointing this out; I can report these internally. But I think you'll have to keep your ignore list around, since it seems unlikely that we would backport fixes for these bugs.

I'm not quite sure what backport means at contemporary Microsoft. What constitutes a new version? Anniversary->Creators (the new "service pack")? Or 10->11?

@zx2c4 These registry types under Class*\Configuration are correct and by design. INF files get compiled into registry state when they're added to the system. The INF is only ever read by the system one time if they're universal, as this one is - from then on, we only reference the registry state. This registry type indicates that it's actually a delete.

@zlockard Indeed I saw it was FLG_ADDREG_DELVAL, but I assumed this would mean actually removing a key, rather than "adding an entry with an undocumented type that implies its absence". I assume you do this so that it's not re-added or something elsewhere? Either way, that's interesting to know. I'll update Go to account for this.

Do you have an REG_* constant name you'd prefer for the 0x200000 type?

These keys get processed every time a media device gets installed (they're part of the device installation state), so we can't just delete them when we read the INF - we have to cache the intent. This logic is internal and there are more types than just this one, so don't fish this type out. For any non-standard types, you should interpret it as REG_BINARY.

Not pretty, but it seems to be working, at least on my machine.

Still failing on the windows-amd64-longtest builder, though (https://build.golang.org/log/89540253578b09ad7362d7748e00c08b9c2d5dd5):

--- FAIL: TestWalkFullRegistry (4.21s)
    registry_test.go:573: value type 1048576 of MODES of LOCAL_MACHINE\DRIVERS\DriverDatabase\DriverPackages\monitor.inf_amd64_c50c5fb00e368b98\Configurations\NonPnPMonitor.Install\Driver failed: <nil>
FAIL
FAIL	golang.org/x/sys/windows/registry	4.251s

Change https://golang.org/cl/203604 mentions this issue: internal/syscall/windows/registry: remove TestWalkFullRegistry due to false assumptions

Change https://golang.org/cl/203605 mentions this issue: windows/registry: remove TestWalkFullRegistry due to false assumptions

@gopherbot, please backport to Go 1.13. This is a test fix. It is needed to resolve a failure on windows-amd64-longtest builder, which could be masking other problems and making releasing Go 1.13.x more difficult.

Backport issue(s) opened: #37826 (for 1.13).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

Change https://golang.org/cl/223237 mentions this issue: [release-branch.go1.13] internal/syscall/windows/registry: remove TestWalkFullRegistry due to false assumptions