Skip to content

registry: a delete operation should be in critical section #39502

@lzhfromustc

Description

@lzhfromustc

Description
authTransport.modReq is a map. Among its 5 usages, 4 are protected by authTransport.mu, but 1 is not protected.

Source Code:
File: registry/session.go Line: 102-164

func (tr *authTransport) RoundTrip(orig *http.Request) (*http.Response, error) {
  ...
  tr.mu.Lock()
  tr.modReq[orig] = req  //PROTECTED
  tr.mu.Unlock()
  ...
  resp, err := tr.RoundTripper.RoundTrip(req)
  if err != nil {
     delete(tr.modReq, orig)  //NOT PROTECTED
     return nil, err
  }
   if len(resp.Header["X-Docker-Token"]) > 0 {
     tr.token = resp.Header["X-Docker-Token"]
  }
  resp.Body = &ioutils.OnEOFReader{
     Rc: resp.Body,
     Fn: func() {
        tr.mu.Lock()
        delete(tr.modReq, orig)  //PROTECTED
        tr.mu.Unlock()
     },
  }
  return resp, nil
}

// CancelRequest cancels an in-flight request by closing its connection.
func (tr *authTransport) CancelRequest(req *http.Request) {
  type canceler interface {
     CancelRequest(*http.Request)
  }
  if cr, ok := tr.RoundTripper.(canceler); ok {
     tr.mu.Lock()
     modReq := tr.modReq[req]  //PROTECTED
     delete(tr.modReq, req)  //PROTECTED
     tr.mu.Unlock()
     cr.CancelRequest(modReq)
  }
}

Suggested Fix:
Add tr.mu.Lock() and tr.mu.Unlock() to protect delete(tr.modReq, orig)
I am requesting an irrelevant PR now. After that one is closed, I can open a PR for this fix.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions