kubeflow / kubeflow/trainer

[Bug]: Potential nil pointer dereference on NumProcPerNode in MPI plugin EnforceMLPolicy

Open Beginner friendly
#4,048 0 comments 0 reactions 0 assignees View on GitHub
kind/bug needs-triage
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.