* [bug report] drm/vkms: Allow to configure multiple CRTCs
@ 2025-08-07 15:53 Dan Carpenter
2025-08-08 11:15 ` José Expósito
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2025-08-07 15:53 UTC (permalink / raw)
To: José Expósito; +Cc: dri-devel
Hello José Expósito,
Commit 600df32dac40 ("drm/vkms: Allow to configure multiple CRTCs")
from Feb 18, 2025 (linux-next), leads to the following Smatch static
checker warning:
drivers/gpu/drm/vkms/tests/vkms_config_test.c:220 vkms_config_test_get_planes() error: 'plane_cfg1' dereferencing possible ERR_PTR()
drivers/gpu/drm/vkms/tests/vkms_config_test.c:258 vkms_config_test_get_crtcs() error: 'crtc_cfg2' dereferencing possible ERR_PTR()
drivers/gpu/drm/vkms/tests/vkms_config_test.c:300 vkms_config_test_get_encoders() error: 'encoder_cfg2' dereferencing possible ERR_PTR()
drivers/gpu/drm/vkms/tests/vkms_config_test.c:345 vkms_config_test_get_connectors() error: 'connector_cfg2' dereferencing possible ERR_PTR()
drivers/gpu/drm/vkms/tests/vkms_config_test.c:672 vkms_config_test_plane_attach_crtc() error: 'overlay_cfg' dereferencing possible ERR_PTR()
drivers/gpu/drm/vkms/tests/vkms_config_test.c:674 vkms_config_test_plane_attach_crtc() error: 'primary_cfg' dereferencing possible ERR_PTR()
drivers/gpu/drm/vkms/tests/vkms_config_test.c:676 vkms_config_test_plane_attach_crtc() error: 'cursor_cfg' dereferencing possible ERR_PTR()
drivers/gpu/drm/vkms/tests/vkms_config_test.c:685 vkms_config_test_plane_attach_crtc() error: 'crtc_cfg' dereferencing possible ERR_PTR()
drivers/gpu/drm/vkms/tests/vkms_config_test.c:746 vkms_config_test_plane_get_possible_crtcs() error: 'crtc_cfg1' dereferencing possible ERR_PTR()
drivers/gpu/drm/vkms/tests/vkms_config_test.c:746 vkms_config_test_plane_get_possible_crtcs() error: 'plane_cfg1' dereferencing possible ERR_PTR()
drivers/gpu/drm/vkms/tests/vkms_config_test.c:748 vkms_config_test_plane_get_possible_crtcs() error: 'crtc_cfg2' dereferencing possible ERR_PTR()
drivers/gpu/drm/vkms/tests/vkms_config_test.c:810 vkms_config_test_encoder_get_possible_crtcs() error: 'crtc_cfg1' dereferencing possible ERR_PTR()
drivers/gpu/drm/vkms/tests/vkms_config_test.c:810 vkms_config_test_encoder_get_possible_crtcs() error: 'encoder_cfg1' dereferencing possible ERR_PTR()
drivers/gpu/drm/vkms/tests/vkms_config_test.c:812 vkms_config_test_encoder_get_possible_crtcs() error: 'crtc_cfg2' dereferencing possible ERR_PTR()
drivers/gpu/drm/vkms/tests/vkms_config_test.c:876 vkms_config_test_connector_get_possible_encoders() error: 'connector_cfg1' dereferencing possible ERR_PTR()
drivers/gpu/drm/vkms/tests/vkms_config_test.c:876 vkms_config_test_connector_get_possible_encoders() error: 'encoder_cfg1' dereferencing possible ERR_PTR()
drivers/gpu/drm/vkms/tests/vkms_config_test.c:878 vkms_config_test_connector_get_possible_encoders() error: 'encoder_cfg2' dereferencing possible ERR_PTR()
drivers/gpu/drm/vkms/tests/vkms_config_test.c
231 static void vkms_config_test_get_crtcs(struct kunit *test)
232 {
233 struct vkms_config *config;
234 struct vkms_config_crtc *crtc_cfg;
235 struct vkms_config_crtc *crtc_cfg1, *crtc_cfg2;
236
237 config = vkms_config_create("test");
238 KUNIT_ASSERT_NOT_ERR_OR_NULL(test, config);
239
240 KUNIT_ASSERT_EQ(test, vkms_config_get_num_crtcs(config), 0);
241 vkms_config_for_each_crtc(config, crtc_cfg)
242 KUNIT_FAIL(test, "Unexpected CRTC");
243
244 crtc_cfg1 = vkms_config_create_crtc(config);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This file has no error checking.
I didn't send an email about it at first because this is just test code so
who cares, but I was recently burned by ignoring errors so now I'm going
through a bunch of old warnings to say that, "Hey, if the author ignores the
error checking that's fine, but I'm in the clear."
245 KUNIT_ASSERT_EQ(test, vkms_config_get_num_crtcs(config), 1);
246 vkms_config_for_each_crtc(config, crtc_cfg) {
247 if (crtc_cfg != crtc_cfg1)
248 KUNIT_FAIL(test, "Unexpected CRTC");
249 }
250
251 crtc_cfg2 = vkms_config_create_crtc(config);
252 KUNIT_ASSERT_EQ(test, vkms_config_get_num_crtcs(config), 2);
253 vkms_config_for_each_crtc(config, crtc_cfg) {
254 if (crtc_cfg != crtc_cfg1 && crtc_cfg != crtc_cfg2)
255 KUNIT_FAIL(test, "Unexpected CRTC");
256 }
257
--> 258 vkms_config_destroy_crtc(config, crtc_cfg2);
259 KUNIT_ASSERT_EQ(test, vkms_config_get_num_crtcs(config), 1);
260 vkms_config_for_each_crtc(config, crtc_cfg) {
261 if (crtc_cfg != crtc_cfg1)
262 KUNIT_FAIL(test, "Unexpected CRTC");
263 }
264
265 vkms_config_destroy(config);
266 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [bug report] drm/vkms: Allow to configure multiple CRTCs 2025-08-07 15:53 [bug report] drm/vkms: Allow to configure multiple CRTCs Dan Carpenter @ 2025-08-08 11:15 ` José Expósito 2025-08-08 14:01 ` Dan Carpenter 0 siblings, 1 reply; 4+ messages in thread From: José Expósito @ 2025-08-08 11:15 UTC (permalink / raw) To: Dan Carpenter; +Cc: dri-devel Hi Dan, Thanks for sharing these warnings. On Thu, Aug 07, 2025 at 06:53:12PM +0300, Dan Carpenter wrote: > Hello José Expósito, > > Commit 600df32dac40 ("drm/vkms: Allow to configure multiple CRTCs") > from Feb 18, 2025 (linux-next), leads to the following Smatch static > checker warning: > > drivers/gpu/drm/vkms/tests/vkms_config_test.c:220 vkms_config_test_get_planes() error: 'plane_cfg1' dereferencing possible ERR_PTR() > drivers/gpu/drm/vkms/tests/vkms_config_test.c:258 vkms_config_test_get_crtcs() error: 'crtc_cfg2' dereferencing possible ERR_PTR() > drivers/gpu/drm/vkms/tests/vkms_config_test.c:300 vkms_config_test_get_encoders() error: 'encoder_cfg2' dereferencing possible ERR_PTR() > drivers/gpu/drm/vkms/tests/vkms_config_test.c:345 vkms_config_test_get_connectors() error: 'connector_cfg2' dereferencing possible ERR_PTR() > drivers/gpu/drm/vkms/tests/vkms_config_test.c:672 vkms_config_test_plane_attach_crtc() error: 'overlay_cfg' dereferencing possible ERR_PTR() > drivers/gpu/drm/vkms/tests/vkms_config_test.c:674 vkms_config_test_plane_attach_crtc() error: 'primary_cfg' dereferencing possible ERR_PTR() > drivers/gpu/drm/vkms/tests/vkms_config_test.c:676 vkms_config_test_plane_attach_crtc() error: 'cursor_cfg' dereferencing possible ERR_PTR() > drivers/gpu/drm/vkms/tests/vkms_config_test.c:685 vkms_config_test_plane_attach_crtc() error: 'crtc_cfg' dereferencing possible ERR_PTR() > drivers/gpu/drm/vkms/tests/vkms_config_test.c:746 vkms_config_test_plane_get_possible_crtcs() error: 'crtc_cfg1' dereferencing possible ERR_PTR() > drivers/gpu/drm/vkms/tests/vkms_config_test.c:746 vkms_config_test_plane_get_possible_crtcs() error: 'plane_cfg1' dereferencing possible ERR_PTR() > drivers/gpu/drm/vkms/tests/vkms_config_test.c:748 vkms_config_test_plane_get_possible_crtcs() error: 'crtc_cfg2' dereferencing possible ERR_PTR() > drivers/gpu/drm/vkms/tests/vkms_config_test.c:810 vkms_config_test_encoder_get_possible_crtcs() error: 'crtc_cfg1' dereferencing possible ERR_PTR() > drivers/gpu/drm/vkms/tests/vkms_config_test.c:810 vkms_config_test_encoder_get_possible_crtcs() error: 'encoder_cfg1' dereferencing possible ERR_PTR() > drivers/gpu/drm/vkms/tests/vkms_config_test.c:812 vkms_config_test_encoder_get_possible_crtcs() error: 'crtc_cfg2' dereferencing possible ERR_PTR() > drivers/gpu/drm/vkms/tests/vkms_config_test.c:876 vkms_config_test_connector_get_possible_encoders() error: 'connector_cfg1' dereferencing possible ERR_PTR() > drivers/gpu/drm/vkms/tests/vkms_config_test.c:876 vkms_config_test_connector_get_possible_encoders() error: 'encoder_cfg1' dereferencing possible ERR_PTR() > drivers/gpu/drm/vkms/tests/vkms_config_test.c:878 vkms_config_test_connector_get_possible_encoders() error: 'encoder_cfg2' dereferencing possible ERR_PTR() > > drivers/gpu/drm/vkms/tests/vkms_config_test.c > 231 static void vkms_config_test_get_crtcs(struct kunit *test) > 232 { > 233 struct vkms_config *config; > 234 struct vkms_config_crtc *crtc_cfg; > 235 struct vkms_config_crtc *crtc_cfg1, *crtc_cfg2; > 236 > 237 config = vkms_config_create("test"); > 238 KUNIT_ASSERT_NOT_ERR_OR_NULL(test, config); > 239 > 240 KUNIT_ASSERT_EQ(test, vkms_config_get_num_crtcs(config), 0); > 241 vkms_config_for_each_crtc(config, crtc_cfg) > 242 KUNIT_FAIL(test, "Unexpected CRTC"); > 243 > 244 crtc_cfg1 = vkms_config_create_crtc(config); > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > This file has no error checking. > > I didn't send an email about it at first because this is just test code so > who cares, but I was recently burned by ignoring errors so now I'm going > through a bunch of old warnings to say that, "Hey, if the author ignores the > error checking that's fine, but I'm in the clear." > > 245 KUNIT_ASSERT_EQ(test, vkms_config_get_num_crtcs(config), 1); While the "crtc_cfg1" pointer is not checked, we check that the number of CRTCs matches the expected value and... > 246 vkms_config_for_each_crtc(config, crtc_cfg) { > 247 if (crtc_cfg != crtc_cfg1) > 248 KUNIT_FAIL(test, "Unexpected CRTC"); > 249 } ...that the stored CRTC matches the one returned by vkms_config_create_crtc(). By program logic, vkms_config_create_crtc() returns an error if allocation fails: ``` struct vkms_config_crtc *vkms_config_create_crtc(struct vkms_config *config) { struct vkms_config_crtc *crtc_cfg; crtc_cfg = kzalloc(sizeof(*crtc_cfg), GFP_KERNEL); if (!crtc_cfg) return ERR_PTR(-ENOMEM); ``` Therefore, the current validations make sure that the pointer is valid. In other places, for example vkms_config_test_connector_get_possible_encoders(), the return value of vkms_config_create_connector/encoder() is indeed not checked. It is not a big deal, since it is test code, but anyways, I'll send a patch checking for return values. Not because I think it can be problematic, but because test code can be used to learn how the API works and I prefer to be explicit about its usage. Best wishes, Jose > 250 > 251 crtc_cfg2 = vkms_config_create_crtc(config); > 252 KUNIT_ASSERT_EQ(test, vkms_config_get_num_crtcs(config), 2); > 253 vkms_config_for_each_crtc(config, crtc_cfg) { > 254 if (crtc_cfg != crtc_cfg1 && crtc_cfg != crtc_cfg2) > 255 KUNIT_FAIL(test, "Unexpected CRTC"); > 256 } > 257 > --> 258 vkms_config_destroy_crtc(config, crtc_cfg2); > 259 KUNIT_ASSERT_EQ(test, vkms_config_get_num_crtcs(config), 1); > 260 vkms_config_for_each_crtc(config, crtc_cfg) { > 261 if (crtc_cfg != crtc_cfg1) > 262 KUNIT_FAIL(test, "Unexpected CRTC"); > 263 } > 264 > 265 vkms_config_destroy(config); > 266 } > > regards, > dan carpenter ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] drm/vkms: Allow to configure multiple CRTCs 2025-08-08 11:15 ` José Expósito @ 2025-08-08 14:01 ` Dan Carpenter 2025-08-11 10:20 ` José Expósito 0 siblings, 1 reply; 4+ messages in thread From: Dan Carpenter @ 2025-08-08 14:01 UTC (permalink / raw) To: José Expósito; +Cc: dri-devel On Fri, Aug 08, 2025 at 01:15:03PM +0200, José Expósito wrote: > > drivers/gpu/drm/vkms/tests/vkms_config_test.c > > 231 static void vkms_config_test_get_crtcs(struct kunit *test) > > 232 { > > 233 struct vkms_config *config; > > 234 struct vkms_config_crtc *crtc_cfg; > > 235 struct vkms_config_crtc *crtc_cfg1, *crtc_cfg2; > > 236 > > 237 config = vkms_config_create("test"); > > 238 KUNIT_ASSERT_NOT_ERR_OR_NULL(test, config); > > 239 > > 240 KUNIT_ASSERT_EQ(test, vkms_config_get_num_crtcs(config), 0); > > 241 vkms_config_for_each_crtc(config, crtc_cfg) > > 242 KUNIT_FAIL(test, "Unexpected CRTC"); > > 243 > > 244 crtc_cfg1 = vkms_config_create_crtc(config); > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > This file has no error checking. > > > > I didn't send an email about it at first because this is just test code so > > who cares, but I was recently burned by ignoring errors so now I'm going > > through a bunch of old warnings to say that, "Hey, if the author ignores the > > error checking that's fine, but I'm in the clear." > > > > 245 KUNIT_ASSERT_EQ(test, vkms_config_get_num_crtcs(config), 1); > > While the "crtc_cfg1" pointer is not checked, we check that the number > of CRTCs matches the expected value and... > Ah yes. That does work... Sorry for the noise. regards, dan carpenter ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] drm/vkms: Allow to configure multiple CRTCs 2025-08-08 14:01 ` Dan Carpenter @ 2025-08-11 10:20 ` José Expósito 0 siblings, 0 replies; 4+ messages in thread From: José Expósito @ 2025-08-11 10:20 UTC (permalink / raw) To: Dan Carpenter; +Cc: dri-devel Hi Dan, On Fri, Aug 08, 2025 at 05:01:35PM +0300, Dan Carpenter wrote: > On Fri, Aug 08, 2025 at 01:15:03PM +0200, José Expósito wrote: > > > drivers/gpu/drm/vkms/tests/vkms_config_test.c > > > 231 static void vkms_config_test_get_crtcs(struct kunit *test) > > > 232 { > > > 233 struct vkms_config *config; > > > 234 struct vkms_config_crtc *crtc_cfg; > > > 235 struct vkms_config_crtc *crtc_cfg1, *crtc_cfg2; > > > 236 > > > 237 config = vkms_config_create("test"); > > > 238 KUNIT_ASSERT_NOT_ERR_OR_NULL(test, config); > > > 239 > > > 240 KUNIT_ASSERT_EQ(test, vkms_config_get_num_crtcs(config), 0); > > > 241 vkms_config_for_each_crtc(config, crtc_cfg) > > > 242 KUNIT_FAIL(test, "Unexpected CRTC"); > > > 243 > > > 244 crtc_cfg1 = vkms_config_create_crtc(config); > > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > This file has no error checking. > > > > > > I didn't send an email about it at first because this is just test code so > > > who cares, but I was recently burned by ignoring errors so now I'm going > > > through a bunch of old warnings to say that, "Hey, if the author ignores the > > > error checking that's fine, but I'm in the clear." > > > > > > 245 KUNIT_ASSERT_EQ(test, vkms_config_get_num_crtcs(config), 1); > > > > While the "crtc_cfg1" pointer is not checked, we check that the number > > of CRTCs matches the expected value and... > > > > Ah yes. That does work... Sorry for the noise. No noise at all, there were other places were the check made sense. I sent a patch fixing them: https://lore.kernel.org/dri-devel/20250811101529.150716-1-jose.exposito89@gmail.com/T/#u Thanks a lot for reporting this issue!! Jose > > regards, > dan carpenter > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-08-11 10:20 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-08-07 15:53 [bug report] drm/vkms: Allow to configure multiple CRTCs Dan Carpenter 2025-08-08 11:15 ` José Expósito 2025-08-08 14:01 ` Dan Carpenter 2025-08-11 10:20 ` José Expósito
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.