[PATCH] Print protocol name of socket descriptors with -yy option

[Masatake YAMATO]

Hi,

> <sys/xattr.h> was introduced in glibc v2.3 about 12 years ago,
> not sure about other libc implementations.
> Can we rely on <sys/xattr.h> and include it unconditionally?

I added the similar code which includes sys/xattr.h to lsof a year ago.
However, I have never heard complains about this inclusion.

>> +static char *
>> +getfdproto(struct tcb *tcp, int fd, char *buf, unsigned bufsize)
>> +{
>> +	char path[sizeof("/proc/%u/fd/%u") + 2 * sizeof(int)*3];
>> +
>> +	if (fd < 0)
>> +		return NULL;
>> +
>> +	sprintf(path, "/proc/%u/fd/%u", tcp->pid, fd);
>> +	if (getxattr(path, "system.sockprotoname", buf, bufsize) <= 0)
>> +		return NULL;
>> +	buf[bufsize - 1] = '\0';
> 
> Masatake, since the code on the kernel side is yours, you should know
> better than anybody else whether the value returned by this syscall is
> null-terminated or not.  If it is surely null-terminated, then we don't
> have to.  If not, then we have to reserve one byte for null, pass
> (bufsize-1) to the syscall, save its return value, and terminate the
> string at the right place.

I just prepared the interface. We know the size of array is 32 bytes:

linux/include/net/sock.h:

	struct proto {
	...
			char			name[32];
	...

Currently, all the protocol implementations use C strings as initial values.
So we can expect 32 bytes C string including NUL terminator.

But Here I used 33 bytes char array and put the NUL terminator at the
endo to avoid crashing of strace caused by broken kernel code which
can be introduced in the future. 

Different subsystem but I found such a broken code in netlink:

     http://comments.gmane.org/gmane.linux.network/262567

I added a code to sparse parser to detect this kind of programming bug.

    https://git.kernel.org/cgit/devel/sparse/chrisl/sparse.git/commit/?id=c3430e57bc57ff40d5ce95ef2d26b8d70346f58f

With sparse I found another broken code.

    http://www.kernelhub.org/?msg=237444&p=2

I'm not sure which is better using 33 bytes array or 32 bytes. However, Put a NUL terminator
at the end in strace side is meaningful.

So I will take the later approach you wrote:

    > If not, then we have to reserve one byte for null, pass
    > (bufsize-1) to the syscall, save its return value, and terminate the
    > string at the right place.

On Wed, 19 Nov 2014 19:10:54 +0300, "Dmitry V. Levin" <ldv at altlinux.org> wrote:
>> +			char proto_buf[PROTO_NAME_LEN];
>> +			char *proto;
>>  			inodenr = strtoul(path + socket_prefix_len, NULL, 10);
>> +			proto = getfdproto(tcp, fd, proto_buf, PROTO_NAME_LEN);
>>  			tprintf("%d<", fd);
>>  			if (!print_sockaddr_by_inode(inodenr))
>> -				tprints(path);
>> +			{
>> +				if (proto)
>> +					tprintf("%s:[%lu]", proto, inodenr);
>> +				else
>> +					tprints(path);
>> +			}
>>  			tprints(">");
> 
> The result of getfdproto call is going to be used only in
> !print_sockaddr_by_inode case, so it would be logical to call
> getfdproto only in that case.

I'll fix this. I thought passing proto to print_sockaddr_by_inode to
utilize it in "inodenr<->TCP connection" dictionary for optimization.

I will resubmit the revised version of patch.
Thank you for reviwing.

Masatake YAMATO