[Bug]: Potential nil pointer dereference on NumProcPerNode in MPI plugin EnforceMLPolicy
- Dominant language
- Go
- Stars
- 2.2k
- Forks
- 1.1k
- Avg merge
- 3d 22h
- Merged PRs (30d)
- 39
Description
### What happened?
### What happened?
In [`pkg/runtime/framework/plugins/mpi/mpi.go`](https://github.com/kubeflow/trainer/blob/master/pkg/runtime/framework/plugins/mpi/mpi.go#L126-L137), the `EnforceMLPolicy` method handles `NumProcPerNode` as follows:
```go
if trainJob.Spec.Trainer != nil && trainJob.Spec.Trainer.NumProcPerNode != nil {
info.RuntimePolicy.MLPolicySource.MPI.NumProcPerNode = trainJob.Spec.Trainer.NumProcPerNode
// If numProcPerNode is set to 1 in runtime, we make it equal to number of GPUs.
} else if *info.RuntimePolicy.MLPolicySource.MPI.NumProcPerNode == 1 {
resourcesPerNode := ptr.Deref(runtime.ExtractResourcePerNodeFromRuntime(info), corev1.ResourceRequirements{})
if jobTrainer := trainJob.Spec.Trainer; jobTrainer != nil && jobTrainer.ResourcesPerNode != nil {
resourcesPerNode = ptr.Deref(jobTrainer.ResourcesPerNode, corev1.ResourceRequirements{})
}
if gpuQ := runtime.GetNumGPUPerNode(&resourcesPerNode); gpuQ > 1 {
info.RuntimePolicy.MLPolicySource.MPI.NumProcPerNode = ptr.To(int32(gpuQ))
}
}
In pkg/apis/trainer/v1alpha1/trainingruntime_types.go, NumProcPerNode in MPIMLPolicySource is an optional pointer (*int32):
type MPIMLPolicySource struct {
// ...
NumProcPerNode *int32 `json:"numProcPerNode,omitempty"`
}
When:
1. trainJob.Spec.Trainer is nil or trainJob.Spec.Trainer.NumProcPerNode is nil, the reconciler falls through to the else if branch.
2. In the else if, it evaluates: *info.RuntimePolicy.MLPolicySource.MPI.NumProcPerNode == 1
3. If info.RuntimePolicy.MLPolicySource.MPI.NumProcPerNode is nil (e.g. if a custom TrainingRuntime or programmatic runtime object was created without explicitly setting numProcPerNode), directly dereferencing *info.RuntimePolicy.MLPolicySource.MPI.NumProcPerNode triggers a panic: panic: runtime error: invalid memory address or nil pointer dereference
### What did you expect to happen?
} else if ptr.Deref(info.RuntimePolicy.MLPolicySource.MPI.NumProcPerNode, 1) == 1 {
### Environment
Kubernetes version:
```bash
$ kubectl version
```
Kubeflow Trainer version:
```bash
$ kubectl get pods -n kubeflow-system -l app.kubernetes.io/name=kubeflow-trainer -o jsonpath="{.items[*].spec.containers[*].image}"
```
Kubeflow Python SDK version:
```bash
$ pip show kubeflow
```
### Impacted by this bug?
Give it a 👍 We prioritize the issues with most 👍
Contributor guide
Research direction
Start in pkg/runtime/framework/plugins/mpi/mpi.go at EnforceMLPolicy and inspect the MPIMLPolicySource definition in pkg/apis/trainer/v1alpha1/trainingruntime_types.go. Trace the nil NumProcPerNode case and verify that custom or programmatic runtimes no longer panic while the existing GPU-based behavior remains unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kubernetes
- Domain
- distributed-systems, infrastructure
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100