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

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Read-only notification of GnuTLS library development activities
GitLab

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.


_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel
Reply | Threaded
Open this post in threaded view
|

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

Read-only notification of GnuTLS library development activities
GitLab

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.


_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel
Reply | Threaded
Open this post in threaded view
|

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

Read-only notification of GnuTLS library development activities
In reply to this post by Read-only notification of GnuTLS library development activities
GitLab

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.


_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel
Reply | Threaded
Open this post in threaded view
|

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

Read-only notification of GnuTLS library development activities
In reply to this post by Read-only notification of GnuTLS library development activities
GitLab

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.


_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel
Reply | Threaded
Open this post in threaded view
|

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

Read-only notification of GnuTLS library development activities
In reply to this post by Read-only notification of GnuTLS library development activities
GitLab

Evgeny Grin commented:

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


_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel
Reply | Threaded
Open this post in threaded view
|

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

Read-only notification of GnuTLS library development activities
In reply to this post by Read-only notification of GnuTLS library development activities
GitLab

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


_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel
Reply | Threaded
Open this post in threaded view
|

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

Read-only notification of GnuTLS library development activities
In reply to this post by Read-only notification of GnuTLS library development activities
GitLab
Re: GnuTLS | Sockets: implement sendmsg()-like function on Win32 (!1377)
GitLab
✓ Merge request was approved (1/1)
 
Merge request icon Merge request !1377 was approved by Avatar Daiki Ueno
 
Project gnutls / GnuTLS
Branch
Branch icon w32_sendmsg
Author
Avatar Evgeny Grin

_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel
Reply | Threaded
Open this post in threaded view
|

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

Read-only notification of GnuTLS library development activities
In reply to this post by Read-only notification of GnuTLS library development activities
GitLab

Daiki Ueno commented:

Looks much better, thanks!


_______________________________________________
Gnutls-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnutls-devel
Reply | Threaded
Open this post in threaded view
|

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

Read-only notification of GnuTLS library development activities
In reply to this post by Read-only notification of GnuTLS library development activities
GitLab

Merge Request !1377 was merged


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