New issue
Advanced search Search tips

Issue 922694 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Today
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 0
Type: Bug



Sign in to add a comment

Assertion in gRPC when creating server connection in garcon

Project Member Reported by jkardatzke@chromium.org, Jan 16 (6 days ago)

Issue description

There's an assertion that's happening in garcon now sometimes where it's getting a negative port number for the gRPC VSOCK server.  This is a new problem, and this kernel change is what triggered it:

https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1404102/

The reason it's affecting gRPC is that in gRPC ports use a signed integer, but in the sockaddr_vm struct ports are an unsigned integer. The kernel patch above randomizes the auto assigned port numbers, and they will go into unsigned range sometimes...causing garcon to assert.

After some discussion, the solution I am going to pursue is having garcon do the port selection itself by attempting to bind until it finds one that is open.  Modifying the kernel patch to keep the ports in the signed integer range is just dirty. Patching gRPC to use unsigned ports will be a large patch and require more maintenance there.  Doing a workaround in garcon seems like the easiest solution that creates the least maintenance.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 19 (4 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/7205446f014bc339e6d21c10f40fff37bc2a41de

commit 7205446f014bc339e6d21c10f40fff37bc2a41de
Author: Jeffrey Kardatzke <jkardatzke@google.com>
Date: Sat Jan 19 04:05:21 2019

vm_tools: garcon: Select vsock port with garcon

This works around a problem that surfaced due to a new kernel patch
where kernel assigned vsock ports now start at a random point in the
unsigned 32 bit integer space. gRPC uses signed 32 bit integers for
ports....so if the kernel picks one with the high bit set, we end up
with assertions in gRPC due to that. This was chosen as the solution
over patching gRPC (big patch, maintenance headaches) or patching the
kernel (the kernel isn't incorrect).

BUG= chromium:922694 
TEST=started multiple containers, each chose a proper port

Change-Id: I7a81c145ca8d44ee3e14ad4db89fae726079b5f0
Reviewed-on: https://chromium-review.googlesource.com/1416414
Commit-Ready: Jeffrey Kardatzke <jkardatzke@google.com>
Tested-by: Jeffrey Kardatzke <jkardatzke@google.com>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>

[modify] https://crrev.com/7205446f014bc339e6d21c10f40fff37bc2a41de/vm_tools/garcon/main.cc

Comment 2 by jkardatzke@chromium.org, Today (12 hours ago)

Status: Fixed (was: Started)

Sign in to add a comment