Re: [gnutls-devel] GnuTLS | Sockets: implement sendmsg()-like function on Win32 (!1377)

[Read-only notification of GnuTLS library development activities]

Evgeny Grin commented on a discussion on lib/system/sockets.c:

	100	+ bufs[to_send_cnt].buf = iovec[to_send_cnt].iov_base;
	101	+ bufs[to_send_cnt].len = (unsigned long)
	102	+ (space_left > ULONG_MAX ?
	103	+ ULONG_MAX : space_left);
	104	+ ovrflwn = true;
	105	+ }
	106	+ else if (iovec[to_send_cnt].iov_len > ULONG_MAX) {
	107	+ bufs[to_send_cnt].buf = iovec[to_send_cnt].iov_base;
	108	+ bufs[to_send_cnt].len = ULONG_MAX;
	109	+ ovrflwn = true;
	110	+ }
	111	+ else {
	112	+ bufs[to_send_cnt].buf = iovec[to_send_cnt].iov_base;
	113	+ bufs[to_send_cnt].len =
	114	+ (unsigned long) iovec[to_send_cnt].iov_len;
	115	+ to_send_bytes += iovec[to_send_cnt].iov_len;

WSASend() can send more than SSIZE_MAX on x32 Windows. The problem is GnuTLS will unable to process this value, as return value could be negative when casted to ssize_t.

—
Reply to this email directly or view it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can unsubscribe from this thread or adjust your notification settings. <script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","action":{"@type":"ViewAction","name":"View Merge request","url":"https://gitlab.com/gnutls/gnutls/-/merge_requests/1377#note_509556788"}}</script>

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel

[Read-only notification of GnuTLS library development activities]

Evgeny Grin commented on a discussion on lib/system/sockets.c:

81	81	}
	82	+
	83	+ssize_t
	84	+system_writev(gnutls_transport_ptr_t ptr, const giovec_t * iovec,
	85	+ int iovec_cnt)
	86	+{
	87	+ WSABUF bufs[iovec_cnt];
	88	+ DWORD bytes_sent;
	89	+ int to_send_cnt;
	90	+ size_t to_send_bytes = 0;
	91	+ bool ovrflwn = false;
	92	+
	93	+ for (to_send_cnt = 0; to_send_cnt < iovec_cnt && !ovrflwn;
	94	+ ++to_send_cnt) {
	95	+ if (to_send_bytes + iovec[to_send_cnt].iov_len > SSIZE_MAX ||
	96	+ iovec[to_send_cnt].iov_len > SSIZE_MAX) {

As per GnuTLS guidelines, I'm trying to avoid comments in code. :)

If iovec[to_send_cnt].iov_len is large enough (close to 2xSSIZE_MAX) then sum of (something + large enough) can warp (overflow) to number less then SSIZE_MAX.

—
Reply to this email directly or view it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can unsubscribe from this thread or adjust your notification settings. <script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","action":{"@type":"ViewAction","name":"View Merge request","url":"https://gitlab.com/gnutls/gnutls/-/merge_requests/1377#note_509556996"}}</script>

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel

[Read-only notification of GnuTLS library development activities]

Evgeny Grin commented on a discussion on lib/system/sockets.c:

	89	+ int to_send_cnt;
	90	+ size_t to_send_bytes = 0;
	91	+ bool ovrflwn = false;
	92	+
	93	+ for (to_send_cnt = 0; to_send_cnt < iovec_cnt && !ovrflwn;
	94	+ ++to_send_cnt) {
	95	+ if (to_send_bytes + iovec[to_send_cnt].iov_len > SSIZE_MAX ||
	96	+ iovec[to_send_cnt].iov_len > SSIZE_MAX) {
	97	+ size_t space_left;
	98	+
	99	+ space_left = (size_t)SSIZE_MAX - to_send_bytes;
	100	+ bufs[to_send_cnt].buf = iovec[to_send_cnt].iov_base;
	101	+ bufs[to_send_cnt].len = (unsigned long)
	102	+ (space_left > ULONG_MAX ?
	103	+ ULONG_MAX : space_left);
	104	+ ovrflwn = true;

It can be replaced with {++to_send_cnt; break;}. Maybe is should be replace for readability.

—
Reply to this email directly or view it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can unsubscribe from this thread or adjust your notification settings. <script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","action":{"@type":"ViewAction","name":"View Merge request","url":"https://gitlab.com/gnutls/gnutls/-/merge_requests/1377#note_509557869"}}</script>

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel

[Read-only notification of GnuTLS library development activities]

Evgeny Grin commented:

@dueno Thanks. Let me know how to improve it.

There are several thing must be handled in this function:

1. The size of each element must be less than ULONG_MAX (actually max value for DWORD). If any element with larger size if found, is must be truncated to ULONG_MAX and no more element must be processed.
2. GnuTLS cannot process successful return value more than SSIZE_MAX, so amount of total sent size must be limited to SSIZE_MAX.

The code is so complicated because ssize_t is variable depending of x32/x64, but size of DWORD is fixed on Win32. To handle both x32 and x64 in uniformed way, code needs to be a bit complicated.

We can shield the second if like:

#if SIZE_MAX > ULONG_MAX
		else if (iovec[to_send_cnt].iov_len > ULONG_MAX) {
			bufs[to_send_cnt].buf = iovec[to_send_cnt].iov_base;
			bufs[to_send_cnt].len = ULONG_MAX;
			ovrflwn = true;
		}
#endif

but it will not make code more readable.

—
Reply to this email directly or view it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can unsubscribe from this thread or adjust your notification settings. <script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","action":{"@type":"ViewAction","name":"View Merge request","url":"https://gitlab.com/gnutls/gnutls/-/merge_requests/1377#note_509565944"}}</script>

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel

[Read-only notification of GnuTLS library development activities]

Evgeny Grin commented:

MR has been updated with more readable version of the code.

—
Reply to this email directly or view it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can unsubscribe from this thread or adjust your notification settings. <script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","action":{"@type":"ViewAction","name":"View Merge request","url":"https://gitlab.com/gnutls/gnutls/-/merge_requests/1377#note_510016976"}}</script>

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel

[Read-only notification of GnuTLS library development activities]

All discussions on Merge Request !1377 were resolved by Daiki Ueno

—
Reply to this email directly or view it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can unsubscribe from this thread or adjust your notification settings. <script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","action":{"@type":"ViewAction","name":"View Merge request","url":"https://gitlab.com/gnutls/gnutls/-/merge_requests/1377"}}</script>

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel

[Read-only notification of GnuTLS library development activities]

Merge request was approved (1/1)

Merge request !1377 was approved by Daiki Ueno

Project

gnutls / GnuTLS

Branch

w32_sendmsg

Author

Evgeny Grin

You're receiving this email because of your account on gitlab.com. Manage all notifications · Help

—
Reply to this email directly or view it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can unsubscribe from this thread or adjust your notification settings. <script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","action":{"@type":"ViewAction","name":"View Merge request","url":"https://gitlab.com/gnutls/gnutls/-/merge_requests/1377"}}</script>

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel

[Read-only notification of GnuTLS library development activities]

Daiki Ueno commented:

Looks much better, thanks!

—
Reply to this email directly or view it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can unsubscribe from this thread or adjust your notification settings. <script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","action":{"@type":"ViewAction","name":"View Merge request","url":"https://gitlab.com/gnutls/gnutls/-/merge_requests/1377#note_510174320"}}</script>

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel

[Read-only notification of GnuTLS library development activities]

Merge Request !1377 was merged

—
Reply to this email directly or view it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can unsubscribe from this thread or adjust your notification settings. <script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","action":{"@type":"ViewAction","name":"View Merge request","url":"https://gitlab.com/gnutls/gnutls/-/merge_requests/1377"}}</script>

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel