Bug 206804 - Inconsistent type handling for sizes in sbuf code
Summary: Inconsistent type handling for sizes in sbuf code
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-bugs mailing list
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2016-01-31 18:20 UTC by CTurt
Modified: 2016-10-07 15:02 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description CTurt 2016-01-31 18:20:50 UTC
Definition of `struct sbuf` in `/sys/sys/sbuf.h`:

struct sbuf {
	char		*s_buf;		/* storage buffer */
	sbuf_drain_func	*s_drain_func;	/* drain function */
	void		*s_drain_arg;	/* user-supplied drain argument */
	int		 s_error;	/* current error code */
	ssize_t		 s_size;	/* size of storage buffer */
	ssize_t		 s_len;		/* current length of string */
#define	SBUF_FIXEDLEN	0x00000000	/* fixed length buffer (default) */
#define	SBUF_AUTOEXTEND	0x00000001	/* automatically extend buffer */
#define	SBUF_INCLUDENUL	0x00000002	/* nulterm byte is counted in len */
#define	SBUF_USRFLAGMSK	0x0000ffff	/* mask of flags the user may specify */
#define	SBUF_DYNAMIC	0x00010000	/* s_buf must be freed */
#define	SBUF_FINISHED	0x00020000	/* set by sbuf_finish() */
#define	SBUF_DYNSTRUCT	0x00080000	/* sbuf must be freed */
#define	SBUF_INSECTION	0x00100000	/* set by sbuf_start_section() */
	int		 s_flags;	/* flags */
	ssize_t		 s_sect_len;	/* current length of section */
};

All sizes and lengths, such as `s_size`, are of type `ssize_t`.

However some functions in `sys/kern/subr_sbuf.c` incorrectly treat these sizes as `int` which could lead to unexpected truncation on platforms where `sizeof(int)` !== `sizeof(ssize_t)`:

struct sbuf *
sbuf_new(struct sbuf *s, char *buf, int length, int flags)
{
    ...
    sbuf_newbuf(s, buf, length, flags);
    ...
}

static struct sbuf *
sbuf_newbuf(struct sbuf *s, char *buf, int length, int flags)
{
    ...
    s->s_size = length;
    ...
}
Comment 3 op 2016-01-31 20:02:16 UTC
These patch breaks the build, see the jenkins log: http://jenkins.hardenedbsd.org:8180/jenkins/job/HardenedBSD-CURRENT-unstable-amd64/lastFailedBuild/consoleFull
Comment 4 CTurt 2016-01-31 20:49:45 UTC
The error is:

/jenkins/workspace/HardenedBSD-CURRENT-unstable-amd64/sy/kern/subr_sbuf.c:226:60: error: format specifies type 'int' but the argument has type 'ssize_t' (aka 'long') [-Werror,-Wformat] ("attempt to create an sbuf of negative length (%d)", length));

Referring to this line:

KASSERT(length >= 0,
	    ("attempt to create an sbuf of negative length (%d)", length));

I find it odd that this would give an error, but the below line doesn't:

	KASSERT(s->s_len < s->s_size,
	    ("wrote past end of sbuf (%d >= %d)", s->s_len, s->s_size));

Regardless, I've committed a new patch to use the `%zd` format specifier here, instead:

https://github.com/HardenedBSD/hardenedBSD-playground/commit/5165b5cc55f09ba357bc1ee41d828ec2864e7d0d.patch