update.go: fix nil pointer risk in Intel RDT initialization

[AntonMoryakov]

Static analysis revealed potential nil dereference when applying RDT configurations. The intelRdtManager instance from NewManager() could be nil but was used without check in Apply() call.

Added explicit nil check that returns error to fail safely rather than panic. This matches error handling patterns used elsewhere in the codebase.

Signed-off-by: Anton Moryakov [email protected]

[AntonMoryakov]

Hi!

While working on this PR, I noticed that the CI fails due to SA5011 reports from staticcheck, but these originate from unrelated test files in the current codebase (not modified in this PR).

Example:

if l.Level != expectedLogs.Level {

This triggers:

SA5011: possible nil pointer dereference

This can be safely resolved using:

if l != nil && l.Level != expectedLogs.Level {

There are similar occurrences in:

• libcontainer/capabilities/capabilities_linux_test.go
• libcontainer/configs/tocpuset_test.go
• libcontainer/system/kernelversion/kernel_linux_test.go

Let me know if you'd like me to submit a cleanup PR to address these 🙌

Thanks! – Anton

[AntonMoryakov]

NACK

A nil dereference is not possible here, because in update.go we call intelrdt.IsCATEnabled and intelrdt.IsMBAEnabled earlier, and if Root returns nil, we'll error out here:

if l3CacheSchema != "" && !intelrdt.IsCATEnabled() {
    return errors.New("Intel RDT/CAT: l3 cache schema is not enabled")
}
  
if memBwSchema != "" && !intelrdt.IsMBAEnabled() {
     return errors.New("Intel RDT/MBA: memory bandwidth schema is not enabled")
}

Thanks for the feedback and the detailed explanation!

You’re absolutely right — after reviewing the call chain and how intelrdt.IsCATEnabled() and intelrdt.IsMBAEnabled() rely on Root(), I agree that this is a false positive from the static analyzer. The nil dereference is not possible in this context, as the failure is already handled earlier during feature checks.

Can we close this PR then? Thanks again for the clarification!

[lifubang]

I agree that this is a false positive from the static analyzer.

Which static analysis tool did you use? I use 'go vet ./...' to test, I can't see any static check errors.

[lifubang]

NACK

In practice, this nil check isn't strictly necessary.

However, as a good coding practice, I think it's worthwhile to include a nil check here. It helps prevent potential errors when refactoring this code in the future.

It's just my own opinion, please feel free to close it.

[AntonMoryakov]

Which static analysis tool did you use? I use 'go vet ./...' to test, I can't see any static check errors.

static analyzers like SVACE

[kolyshkin]

It looks like this one should be closed. If there's something that needs to be done, please open a new PR.