A couple weeks ago I found the bug 87ebccf97f54 ('staging: lustre:
validate size in ll_setxattr()') by manual audit but now I have written
a Smatch check to find bugs like that automatically. The results look
to be fairly high quality generally except in lustre. These have a
potential security impact but I can't untangle lustre enough to know
what to do about it.
Most of the warnings are from lustre. Then number at the end is what
Smatch thinks is the possible minimum length of the buffer.
A size of zero can mean either the ZERO_SIZE_POINTER or NULL.
drivers/staging/lustre/lustre/lov/lov_obd.c:1748 lov_fiemap() warn: is 'key' large enough for 'struct ll_fiemap_info_key'? 6
drivers/staging/lustre/lustre/lov/lov_obd.c:1974 lov_get_info() warn: is 'key' large enough for 'struct <anon>'? 14
The following are all from the ->o_iocontrol() pointer functions.
Obviously negative lengths like s32min are impossible. Part of the
problem is that lustre calls these things recursively where one
->o_iocontrol() function calls a different ->o_iocontrol() function.
Smatch gets lost and I both get completely lost.
Some of the callers use obd_ioctl_getdata() which insists on a buffer
592-8192 bytes large. That is great and it works. Other callers do use
different size buffers though such as 24 or 36 bytes. Several pass NULL
buffers.
drivers/staging/lustre/lustre/lov/lov_obd.c:1502 lov_iocontrol() warn: is 'karg' large enough for 'struct if_quotactl'? 0
drivers/staging/lustre/lustre/obdecho/echo_client.c:1863 echo_client_iocontrol() warn: is 'karg' large enough for 'struct obd_ioctl_data'? s32min
drivers/staging/lustre/lustre/osc/osc_request.c:2715 osc_iocontrol() warn: is 'karg' large enough for 'struct obd_ioctl_data'? s32min
drivers/staging/lustre/lustre/osc/osc_request.c:2721 osc_iocontrol() warn: is 'karg' large enough for 'struct obd_ioctl_data'? s32min
drivers/staging/lustre/lustre/lmv/lmv_obd.c:961 lmv_iocontrol() warn: is 'karg' large enough for 'struct if_quotactl'? 0
drivers/staging/lustre/lustre/lmv/lmv_obd.c:1008 lmv_iocontrol() warn: is 'karg' large enough for 'struct ioc_changelog'? s32min
drivers/staging/lustre/lustre/lmv/lmv_obd.c:1035 lmv_iocontrol() warn: is 'karg' large enough for 'struct md_op_data'? s32min
drivers/staging/lustre/lustre/lmv/lmv_obd.c:1049 lmv_iocontrol() warn: is 'karg' large enough for 'struct hsm_progress_kernel'? s32min
drivers/staging/lustre/lustre/lmv/lmv_obd.c:1058 lmv_iocontrol() warn: is 'karg' large enough for 'struct hsm_user_request'? s32min
drivers/staging/lustre/lustre/lmv/lmv_obd.c:1106 lmv_iocontrol() warn: is 'karg' large enough for 'struct md_op_data'? s32min
drivers/staging/lustre/lustre/lmv/lmv_obd.c:1126 lmv_iocontrol() warn: is 'karg' large enough for 'struct lustre_kernelcomm'? s32min
drivers/staging/lustre/lustre/mdc/mdc_request.c:1846 mdc_iocontrol() warn: is 'karg' large enough for 'struct ioc_changelog'? s32min
drivers/staging/lustre/lustre/mdc/mdc_request.c:1880 mdc_iocontrol() warn: is 'karg' large enough for 'struct obd_ioctl_data'? s32min
drivers/staging/lustre/lustre/mdc/mdc_request.c:1886 mdc_iocontrol() warn: is 'karg' large enough for 'struct obd_ioctl_data'? s32min
drivers/staging/lustre/lustre/mdc/mdc_request.c:1943 mdc_iocontrol() warn: is 'karg' large enough for 'struct if_quotactl'? 0
Maybe the real problem is that we have a lot of functions called
->iocontrol() but they behave in totally different ways. The take
different kinds of pointers for arguments but they are casted to void
pointer so they look the same. Sometimes it's ok to pass a NULL pointer
and other times it will cause a crash. They all take a length parameter
and sometimes you have to check that it's valid but other times you
trust the caller. Basically they are completely different functions
except the name is the same.
TODO-list: 2014-11-06: lustre: audit buffer sizes
TODO-list: 2014-11-06: lustre: get rid of the ->o_iocontrol() pointers
because casting pointers to void is not a real abstraction layer.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
