Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sql: granting a global priv to a user for the first time can access priv cache without sufficient locking #142992

Open
msbutler opened this issue Mar 17, 2025 · 1 comment
Open

sql: granting a global priv to a user for the first time can access priv cache without sufficient locking #142992

msbutler opened this issue Mar 17, 2025 · 1 comment
Assignees
@Dedej-Bergin
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@msbutler
Copy link
Collaborator

msbutler commented Mar 17, 2025 •
edited by cockroach-jira-scripts
Loading

An unrelated PR surfaced an interesting race (stack below) which suggests that the global privilege descriptor cache is not locking objects sufficiently before modification. One goroutine attempted to update a priv descriptor in the cache here, while a different goroutine was authorizing a request using the same descriptor here.

More context in slack thread https://cockroachlabs.slack.com/archives/C04N0AS14CT/p1741980496915059

I'll let sql-foundations determine how many branches this bug affects.

No pass/skip/fail event found for test
=== RUN   TestUserPrivileges/db-create-replication-dest
==================
WARNING: DATA RACE
Read at 0x00c00a8d6070 by goroutine 11866:
  github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb.PrivilegeDescriptor.findUserIndex.func1()
      pkg/sql/catalog/catpb/privilege.go:59 +0x6d
  sort.Search()
      GOROOT/src/sort/search.go:65 +0x53
  github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb.PrivilegeDescriptor.findUserIndex()
      pkg/sql/catalog/catpb/privilege.go:58 +0x11b
  github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb.PrivilegeDescriptor.FindUser()
      pkg/sql/catalog/catpb/privilege.go:70 +0x16e
  github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb.PrivilegeDescriptor.CheckPrivilege()
      pkg/sql/catalog/catpb/privilege.go:512 +0xd8
  github.com/cockroachdb/cockroach/pkg/sql.(*planner).HasPrivilege.func1()
      pkg/sql/authorization.go:171 +0x14d
  github.com/cockroachdb/cockroach/pkg/sql.(*planner).checkRolePredicate()
      pkg/sql/authorization.go:430 +0x6f
  github.com/cockroachdb/cockroach/pkg/sql.(*planner).HasPrivilege()
      pkg/sql/authorization.go:169 +0x5f1
  github.com/cockroachdb/cockroach/pkg/sql.(*planner).HasGlobalPrivilegeOrRoleOption()
      pkg/sql/authorization.go:695 +0x1a4
  github.com/cockroachdb/cockroach/pkg/jobs/jobsauth.GetGlobalJobPrivileges()
      pkg/jobs/jobsauth/authorization.go:96 +0x4e
  github.com/cockroachdb/cockroach/pkg/crosscluster/producer.(*replicationStreamManagerImpl).AuthorizeViaJob()
      pkg/crosscluster/producer/replication_manager.go:439 +0x12f
  github.com/cockroachdb/cockroach/pkg/sql/sem/builtins.init.func835()
      pkg/sql/sem/builtins/replication_builtins.go:209 +0x35a
  github.com/cockroachdb/cockroach/pkg/sql/sem/eval.(*evaluator).EvalFuncExpr()
      pkg/sql/sem/eval/expr.go:503 +0x2af
  github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FuncExpr).Eval()
      bazel-out/k8-fastbuild/bin/pkg/sql/sem/tree/eval_expr_generated.go:287 +0x59
  github.com/cockroachdb/cockroach/pkg/sql/sem/eval.Expr()
      pkg/sql/sem/eval/expr.go:21 +0x4c1
  github.com/cockroachdb/cockroach/pkg/sql.(*valuesNode).startExec()
      pkg/sql/values.go:86 +0x48e
  github.com/cockroachdb/cockroach/pkg/sql.startExec()
      pkg/sql/plan.go:564 +0x365
  github.com/cockroachdb/cockroach/pkg/sql.(*planNodeToRowSource).Start()
      pkg/sql/plan_node_to_row_source.go:167 +0x225
  github.com/cockroachdb/cockroach/pkg/sql/colflow.(*FlowCoordinator).Start.func1()
      pkg/sql/colflow/flow_coordinator.go:111 +0x6e
  github.com/cockroachdb/cockroach/pkg/sql/colexecerror.CatchVectorizedRuntimeError()
      pkg/sql/colexecerror/error.go:162 +0x7c
  github.com/cockroachdb/cockroach/pkg/sql/colflow.(*FlowCoordinator).Start()
      pkg/sql/colflow/flow_coordinator.go:110 +0xd2
  github.com/cockroachdb/cockroach/pkg/sql/execinfra.(*ProcessorBaseNoHelper).Run()
      pkg/sql/execinfra/processorsbase.go:726 +0x70
  github.com/cockroachdb/cockroach/pkg/sql/colflow.(*FlowCoordinator).Run()
      <autogenerated>:1 +0x5d
  github.com/cockroachdb/cockroach/pkg/sql/flowinfra.(*FlowBase).Run()
      pkg/sql/flowinfra/flow.go:574 +0x77c
  github.com/cockroachdb/cockroach/pkg/sql/colflow.(*vectorizedFlow).Run()
      pkg/sql/colflow/vectorized_flow.go:301 +0x39a
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).Run()
      pkg/sql/distsql_running.go:975 +0x18f5
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).PlanAndRun()
      pkg/sql/distsql_running.go:2078 +0x331
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).PlanAndRunAll.func3()
      pkg/sql/distsql_running.go:1782 +0x1b0
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).PlanAndRunAll()
      pkg/sql/distsql_running.go:1785 +0x373
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execWithDistSQLEngine()
      pkg/sql/conn_executor_exec.go:3448 +0xbb0
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).dispatchToExecutionEngine()
      pkg/sql/conn_executor_exec.go:2980 +0x2452
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmtInOpenState()
      pkg/sql/conn_executor_exec.go:1077 +0x5d74
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt.func2()
      pkg/sql/conn_executor_exec.go:170 +0x184
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execWithProfiling()
      pkg/sql/conn_executor_exec.go:4464 +0x539
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt()
      pkg/sql/conn_executor_exec.go:169 +0xdbd
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execPortal()
      pkg/sql/conn_executor_exec.go:320 +0x784
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd.func2()
      pkg/sql/conn_executor.go:2537 +0x1291
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd()
      pkg/sql/conn_executor.go:2539 +0xbef
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run()
      pkg/sql/conn_executor.go:2347 +0x3ea
  github.com/cockroachdb/cockroach/pkg/sql.(*Server).ServeConn()
      pkg/sql/conn_executor.go:1018 +0x216
  github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*conn).processCommands()
      pkg/sql/pgwire/conn.go:252 +0x706
  github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*Server).serveImpl.func4()
      pkg/sql/pgwire/server.go:1197 +0x21c

Previous write at 0x00c00a8d6070 by goroutine 3566:
  github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb.(*PrivilegeDescriptor).Grant()
      pkg/sql/catalog/catpb/privilege.go:269 +0x1b5
  github.com/cockroachdb/cockroach/pkg/sql.(*changeNonDescriptorBackedPrivilegesNode).startExec()
      pkg/sql/grant_revoke_system.go:76 +0x1b66
  github.com/cockroachdb/cockroach/pkg/sql.startExec()
      pkg/sql/plan.go:564 +0x365
  github.com/cockroachdb/cockroach/pkg/sql.(*planNodeToRowSource).Start()
      pkg/sql/plan_node_to_row_source.go:167 +0x225
  github.com/cockroachdb/cockroach/pkg/sql/colexec.(*Columnarizer).Init()
      pkg/sql/colexec/columnarizer.go:178 +0x19e
  github.com/cockroachdb/cockroach/pkg/sql/colexec.(*invariantsChecker).Init()
      pkg/sql/colexec/invariants_checker.go:62 +0x97
  github.com/cockroachdb/cockroach/pkg/sql/colflow.(*batchInfoCollector).init()
      pkg/sql/colflow/stats.go:96 +0x77
  github.com/cockroachdb/cockroach/pkg/sql/colflow.(*batchInfoCollector).init-fm()
      <autogenerated>:1 +0x17
  github.com/cockroachdb/cockroach/pkg/sql/colexecerror.CatchVectorizedRuntimeError()
      pkg/sql/colexecerror/error.go:162 +0x7c
  github.com/cockroachdb/cockroach/pkg/sql/colflow.(*batchInfoCollector).Init()
      pkg/sql/colflow/stats.go:105 +0xde
  github.com/cockroachdb/cockroach/pkg/sql/colflow.(*vectorizedStatsCollectorImpl).Init()
      <autogenerated>:1 +0x45
  github.com/cockroachdb/cockroach/pkg/sql/colflow.(*BatchFlowCoordinator).Run.(*BatchFlowCoordinator).init.func2()
      pkg/sql/colflow/flow_coordinator.go:236 +0x65
  github.com/cockroachdb/cockroach/pkg/sql/colexecerror.CatchVectorizedRuntimeError()
      pkg/sql/colexecerror/error.go:162 +0x7c
  github.com/cockroachdb/cockroach/pkg/sql/colflow.(*BatchFlowCoordinator).init()
      pkg/sql/colflow/flow_coordinator.go:235 +0x1ee
  github.com/cockroachdb/cockroach/pkg/sql/colflow.(*BatchFlowCoordinator).Run()
      pkg/sql/colflow/flow_coordinator.go:269 +0x1ef
  github.com/cockroachdb/cockroach/pkg/sql/colflow.(*vectorizedFlow).Run()
      pkg/sql/colflow/vectorized_flow.go:316 +0x345
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).Run()
      pkg/sql/distsql_running.go:975 +0x18f5
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).PlanAndRun()
      pkg/sql/distsql_running.go:2078 +0x331
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).PlanAndRunAll.func3()
      pkg/sql/distsql_running.go:1782 +0x1b0
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).PlanAndRunAll()
      pkg/sql/distsql_running.go:1785 +0x373
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execWithDistSQLEngine()
      pkg/sql/conn_executor_exec.go:3448 +0xbb0
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).dispatchToExecutionEngine()
      pkg/sql/conn_executor_exec.go:2980 +0x2452
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).dispatchToExecutionEngine()
      pkg/sql/conn_executor_exec.go:2851 +0x1253
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmtInOpenState()
      pkg/sql/conn_executor_exec.go:1077 +0x5d74
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt.func2()
      pkg/sql/conn_executor_exec.go:170 +0x184
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execWithProfiling()
      pkg/sql/conn_executor_exec.go:4464 +0x539
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt()
      pkg/sql/conn_executor_exec.go:169 +0xdbd
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd.func1()
      pkg/sql/conn_executor.go:2425 +0x864
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd()
      pkg/sql/conn_executor.go:2430 +0xfaf
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run()
      pkg/sql/conn_executor.go:2347 +0x3ea
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run()
      pkg/sql/conn_executor.go:2347 +0x3ea
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run()
      pkg/sql/conn_executor.go:2347 +0x3ea
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run()
      pkg/sql/conn_executor.go:2347 +0x3ea
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run()
      pkg/sql/conn_executor.go:2347 +0x3ea
  github.com/cockroachdb/cockroach/pkg/sql.(*Server).ServeConn()
      pkg/sql/conn_executor.go:1018 +0x216
  github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*conn).processCommands()
      pkg/sql/pgwire/conn.go:252 +0x706
  github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*Server).serveImpl.func4()
      pkg/sql/pgwire/server.go:1197 +0x21c

Jira issue: CRDB-48617
@msbutler msbutler added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Mar 17, 2025
@blathers-crl blathers[crl]
Copy link

blathers-crl bot commented Mar 17, 2025

Hi @msbutler, please add branch-* labels to identify which branch(es) this C-bug affects.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@msbutler msbutler added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Dedej-Bergin Dedej-Bergin

Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@msbutler @Dedej-Bergin