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.

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

gRPC Server's IVI-Dance has race conditions #247

Closed
marcoskirsch opened this issue Jul 6, 2021 · 0 comments · Fixed by #309
Closed

gRPC Server's IVI-Dance has race conditions #247

marcoskirsch opened this issue Jul 6, 2021 · 0 comments · Fixed by #309
Assignees

Comments

@marcoskirsch
Copy link
Member

marcoskirsch commented Jul 6, 2021

See code in https://github.com/ni/grpc-device/blob/main/generated/nidcpower/nidcpower_service.cpp#L2400

In IVI based APIs, the way clients retrieve strings is by calling the function to get the size, allocating, then calling again with the actual buffer (aka "the IVI Dance").

It is being done incorrectly: the recursive session lock isn't grabbed, so it is possible for the size of the string to grow between the time in which the size is queried and the time in which the string is copied onto the client buffer.

There are two ways to fix it:

  1. Acquire the session lock. This is how NI's own C# and Python APIs do it internally.
  2. Keep growing the buffer in a loop until it's big enough, which is usually just the one time. This is how NI's own Digital Pattern Editor does it internaly.

AB#1579918

@gregstoll gregstoll self-assigned this Aug 16, 2021
@gregstoll gregstoll linked a pull request Aug 18, 2021 that will close this issue
gregstoll pushed a commit that referenced this issue Aug 19, 2021
Fixes a race condition if the size of a buffer changes between the initial sizing call and the "real" call afterwards.

This code assumes that if you pass in too small a buffer to a vanilla `ivi-dance` method it either returns the necessary size, or returns `kErrorReadBufferTooSmall`. For `ivi-dance-with-a-twist` methods, passing in too small a buffer will return `kErrorReadBufferTooSmall`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants