| Message ID | 20220517110903.25491-1-oneukum@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | airspy: respect the DMA coherency rules | expand |
Hi Oliver,
I love your patch! Perhaps something to improve:
[auto build test WARNING on media-tree/master]
[also build test WARNING on v5.18-rc7 next-20220517]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Oliver-Neukum/airspy-respect-the-DMA-coherency-rules/20220517-191203
base: git://linuxtv.org/media_tree.git master
config: i386-randconfig-a006-20220516 (https://download.01.org/0day-ci/archive/20220518/202205180516.aJrehmYZ-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 853fa8ee225edf2d0de94b0dcbd31bea916e825e)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/1001f475c1972ae2ab2be36a64d723f12c56786d
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Oliver-Neukum/airspy-respect-the-DMA-coherency-rules/20220517-191203
git checkout 1001f475c1972ae2ab2be36a64d723f12c56786d
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/media/usb/airspy/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/media/usb/airspy/airspy.c:979:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!buf)
^~~~
drivers/media/usb/airspy/airspy.c:1082:9: note: uninitialized use occurs here
return ret;
^~~
drivers/media/usb/airspy/airspy.c:979:2: note: remove the 'if' if its condition is always false
if (!buf)
^~~~~~~~~
drivers/media/usb/airspy/airspy.c:976:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!s->buf)
^~~~~~~
drivers/media/usb/airspy/airspy.c:1082:9: note: uninitialized use occurs here
return ret;
^~~
drivers/media/usb/airspy/airspy.c:976:2: note: remove the 'if' if its condition is always false
if (!s->buf)
^~~~~~~~~~~~
drivers/media/usb/airspy/airspy.c:966:9: note: initialize the variable 'ret' to silence this warning
int ret;
^
= 0
2 warnings generated.
vim +979 drivers/media/usb/airspy/airspy.c
961
962 static int airspy_probe(struct usb_interface *intf,
963 const struct usb_device_id *id)
964 {
965 struct airspy *s;
966 int ret;
967 u8 u8tmp, *buf = NULL;
968
969 s = kzalloc(sizeof(struct airspy), GFP_KERNEL);
970 if (s == NULL) {
971 dev_err(&intf->dev, "Could not allocate memory for state\n");
972 return -ENOMEM;
973 }
974
975 s->buf = kzalloc(BUF_SIZE, GFP_KERNEL);
976 if (!s->buf)
977 goto err_free_mem;
978 buf = kzalloc(BUF_SIZE, GFP_KERNEL);
> 979 if (!buf)
980 goto err_free_mem;
981
982 mutex_init(&s->v4l2_lock);
983 mutex_init(&s->vb_queue_lock);
984 spin_lock_init(&s->queued_bufs_lock);
985 INIT_LIST_HEAD(&s->queued_bufs);
986 s->dev = &intf->dev;
987 s->udev = interface_to_usbdev(intf);
988 s->f_adc = bands[0].rangelow;
989 s->f_rf = bands_rf[0].rangelow;
990 s->pixelformat = formats[0].pixelformat;
991 s->buffersize = formats[0].buffersize;
992
993 /* Detect device */
994 ret = airspy_ctrl_msg(s, CMD_BOARD_ID_READ, 0, 0, &u8tmp, 1);
995 if (ret == 0)
996 ret = airspy_ctrl_msg(s, CMD_VERSION_STRING_READ, 0, 0,
997 buf, BUF_SIZE);
998 if (ret) {
999 dev_err(s->dev, "Could not detect board\n");
1000 goto err_free_mem;
1001 }
1002
1003 buf[BUF_SIZE - 1] = '\0';
1004
1005 dev_info(s->dev, "Board ID: %02x\n", u8tmp);
1006 dev_info(s->dev, "Firmware version: %s\n", buf);
1007
1008 /* Init videobuf2 queue structure */
1009 s->vb_queue.type = V4L2_BUF_TYPE_SDR_CAPTURE;
1010 s->vb_queue.io_modes = VB2_MMAP | VB2_USERPTR | VB2_READ;
1011 s->vb_queue.drv_priv = s;
1012 s->vb_queue.buf_struct_size = sizeof(struct airspy_frame_buf);
1013 s->vb_queue.ops = &airspy_vb2_ops;
1014 s->vb_queue.mem_ops = &vb2_vmalloc_memops;
1015 s->vb_queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
1016 ret = vb2_queue_init(&s->vb_queue);
1017 if (ret) {
1018 dev_err(s->dev, "Could not initialize vb2 queue\n");
1019 goto err_free_mem;
1020 }
1021
1022 /* Init video_device structure */
1023 s->vdev = airspy_template;
1024 s->vdev.queue = &s->vb_queue;
1025 s->vdev.queue->lock = &s->vb_queue_lock;
1026 video_set_drvdata(&s->vdev, s);
1027
1028 /* Register the v4l2_device structure */
1029 s->v4l2_dev.release = airspy_video_release;
1030 ret = v4l2_device_register(&intf->dev, &s->v4l2_dev);
1031 if (ret) {
1032 dev_err(s->dev, "Failed to register v4l2-device (%d)\n", ret);
1033 goto err_free_mem;
1034 }
1035
1036 /* Register controls */
1037 v4l2_ctrl_handler_init(&s->hdl, 5);
1038 s->lna_gain_auto = v4l2_ctrl_new_std(&s->hdl, &airspy_ctrl_ops,
1039 V4L2_CID_RF_TUNER_LNA_GAIN_AUTO, 0, 1, 1, 0);
1040 s->lna_gain = v4l2_ctrl_new_std(&s->hdl, &airspy_ctrl_ops,
1041 V4L2_CID_RF_TUNER_LNA_GAIN, 0, 14, 1, 8);
1042 v4l2_ctrl_auto_cluster(2, &s->lna_gain_auto, 0, false);
1043 s->mixer_gain_auto = v4l2_ctrl_new_std(&s->hdl, &airspy_ctrl_ops,
1044 V4L2_CID_RF_TUNER_MIXER_GAIN_AUTO, 0, 1, 1, 0);
1045 s->mixer_gain = v4l2_ctrl_new_std(&s->hdl, &airspy_ctrl_ops,
1046 V4L2_CID_RF_TUNER_MIXER_GAIN, 0, 15, 1, 8);
1047 v4l2_ctrl_auto_cluster(2, &s->mixer_gain_auto, 0, false);
1048 s->if_gain = v4l2_ctrl_new_std(&s->hdl, &airspy_ctrl_ops,
1049 V4L2_CID_RF_TUNER_IF_GAIN, 0, 15, 1, 0);
1050 if (s->hdl.error) {
1051 ret = s->hdl.error;
1052 dev_err(s->dev, "Could not initialize controls\n");
1053 goto err_free_controls;
1054 }
1055
1056 v4l2_ctrl_handler_setup(&s->hdl);
1057
1058 s->v4l2_dev.ctrl_handler = &s->hdl;
1059 s->vdev.v4l2_dev = &s->v4l2_dev;
1060 s->vdev.lock = &s->v4l2_lock;
1061 s->vdev.device_caps = V4L2_CAP_SDR_CAPTURE | V4L2_CAP_STREAMING |
1062 V4L2_CAP_READWRITE | V4L2_CAP_TUNER;
1063
1064 ret = video_register_device(&s->vdev, VFL_TYPE_SDR, -1);
1065 if (ret) {
1066 dev_err(s->dev, "Failed to register as video device (%d)\n",
1067 ret);
1068 goto err_free_controls;
1069 }
1070 dev_info(s->dev, "Registered as %s\n",
1071 video_device_node_name(&s->vdev));
1072 dev_notice(s->dev, "SDR API is still slightly experimental and functionality changes may follow\n");
1073 return 0;
1074
1075 err_free_controls:
1076 v4l2_ctrl_handler_free(&s->hdl);
1077 v4l2_device_unregister(&s->v4l2_dev);
1078 err_free_mem:
1079 kfree(buf);
1080 kfree(s->buf);
1081 kfree(s);
1082 return ret;
1083 }
1084
diff --git a/drivers/media/usb/airspy/airspy.c b/drivers/media/usb/airspy/airspy.c index d568452618d1..ed560e594efb 100644 --- a/drivers/media/usb/airspy/airspy.c +++ b/drivers/media/usb/airspy/airspy.c @@ -123,7 +123,7 @@ struct airspy { /* USB control message buffer */ #define BUF_SIZE 128 - u8 buf[BUF_SIZE]; + u8 *buf; /* Current configuration */ unsigned int f_adc; @@ -856,6 +856,7 @@ static void airspy_video_release(struct v4l2_device *v) v4l2_ctrl_handler_free(&s->hdl); v4l2_device_unregister(&s->v4l2_dev); + kfree(s->buf); kfree(s); } @@ -963,7 +964,7 @@ static int airspy_probe(struct usb_interface *intf, { struct airspy *s; int ret; - u8 u8tmp, buf[BUF_SIZE]; + u8 u8tmp, *buf = NULL; s = kzalloc(sizeof(struct airspy), GFP_KERNEL); if (s == NULL) { @@ -971,6 +972,13 @@ static int airspy_probe(struct usb_interface *intf, return -ENOMEM; } + s->buf = kzalloc(BUF_SIZE, GFP_KERNEL); + if (!s->buf) + goto err_free_mem; + buf = kzalloc(BUF_SIZE, GFP_KERNEL); + if (!buf) + goto err_free_mem; + mutex_init(&s->v4l2_lock); mutex_init(&s->vb_queue_lock); spin_lock_init(&s->queued_bufs_lock); @@ -1068,6 +1076,8 @@ static int airspy_probe(struct usb_interface *intf, v4l2_ctrl_handler_free(&s->hdl); v4l2_device_unregister(&s->v4l2_dev); err_free_mem: + kfree(buf); + kfree(s->buf); kfree(s); return ret; }
If we want to avoid memory corruption on incoherent architectures, buffers for DMA must not reside - on the stack - embedded within other structures Allocate them separately. Signed-off-by: Oliver Neukum <oneukum@suse.com> --- drivers/media/usb/airspy/airspy.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)