Skip to content

CLBlast: Fix temporary buffer size for f16 conversion (wsize) #3603

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

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

shibe2
Copy link
Contributor

@shibe2 shibe2 commented Oct 12, 2023

In ggml_cl_mul_mat_f16, wdata is used for conversion of both src1 and dst, but only the size of src1 was considered. When dst required more space, it could lead to buffer overflow. Crashes reported in #2378 and #2736 may have been caused by this.

For 3D and 4D tensors, the size to fit the whole src1 was requested, but only one 2D slice is processed at a time, so the requirement can be reduced.

Tested with AddressSanitizer.

Fix buffer overflow.
Reduce the size to fit just one 2D slice.
Assert sufficient size.
Copy link
Member

@ggerganov ggerganov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We haven't seen confirmation that his fixes the referenced issues, but I think it's safe to merge

@shibe2 shibe2 merged commit 40e5ce0 into ggml-org:master Oct 17, 2023
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 this pull request may close these issues.

3 participants