On 3/24/2023 02:12, Dan Carpenter wrote:
Hello John Harrison,
The patch 9bbba0667f37: "drm/i915/guc: Use GuC submission API version
number" from Nov 29, 2022, leads to the following Smatch static
checker warning:
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:821 intel_uc_fw_fetch()
warn: passing zero to 'ERR_PTR'
Doh! Yes. That is a bug. It's an error path that should never be hit -
it means a firmware file has been released with a version number that
breaks the spec or is simply corrupted data. So it's a low risk bug but
it should still be fixed. I'll post a patch...
Thanks,
John.
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
709 int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
710 {
711 struct intel_gt *gt = __uc_fw_to_gt(uc_fw);
712 struct drm_i915_private *i915 = gt->i915;
713 struct intel_uc_fw_file file_ideal;
714 struct drm_i915_gem_object *obj;
715 const struct firmware *fw = NULL;
716 bool old_ver = false;
717 int err;
718
719 GEM_BUG_ON(!gt->wopcm.size);
720 GEM_BUG_ON(!intel_uc_fw_is_enabled(uc_fw));
721
722 err = i915_inject_probe_error(i915, -ENXIO);
723 if (err)
724 goto fail;
725
726 __force_fw_fetch_failures(uc_fw, -EINVAL);
727 __force_fw_fetch_failures(uc_fw, -ESTALE);
728
729 err = try_firmware_load(uc_fw, &fw);
730 memcpy(&file_ideal, &uc_fw->file_wanted, sizeof(file_ideal));
731
732 /* Any error is terminal if overriding. Don't bother searching for older versions */
733 if (err && intel_uc_fw_is_overridden(uc_fw))
734 goto fail;
735
736 while (err == -ENOENT) {
737 old_ver = true;
738
739 __uc_fw_auto_select(i915, uc_fw);
740 if (!uc_fw->file_selected.path) {
741 /*
742 * No more options! But set the path back to something
743 * valid just in case it gets dereferenced.
744 */
745 uc_fw->file_selected.path = file_ideal.path;
746
747 /* Also, preserve the version that was really wanted */
748 memcpy(&uc_fw->file_wanted, &file_ideal, sizeof(uc_fw->file_wanted));
749 break;
750 }
751
752 err = try_firmware_load(uc_fw, &fw);
753 }
754
755 if (err)
756 goto fail;
757
758 err = check_fw_header(gt, fw, uc_fw);
759 if (err)
760 goto fail;
761
762 if (uc_fw->type == INTEL_UC_FW_TYPE_GUC && !guc_check_version_range(uc_fw))
763 goto fail;
^^^^^^^^^
Should "err" be set on this path?
764
765 if (uc_fw->file_wanted.ver.major && uc_fw->file_selected.ver.major) {
766 /* Check the file's major version was as it claimed */
767 if (uc_fw->file_selected.ver.major != uc_fw->file_wanted.ver.major) {
768 gt_notice(gt, "%s firmware %s: unexpected version: %u.%u != %u.%u\n",
769 intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path,
770 uc_fw->file_selected.ver.major, uc_fw->file_selected.ver.minor,
771 uc_fw->file_wanted.ver.major, uc_fw->file_wanted.ver.minor);
772 if (!intel_uc_fw_is_overridden(uc_fw)) {
773 err = -ENOEXEC;
774 goto fail;
775 }
776 } else {
777 if (uc_fw->file_selected.ver.minor < uc_fw->file_wanted.ver.minor)
778 old_ver = true;
779 }
780 }
781
782 if (old_ver && uc_fw->file_selected.ver.major) {
783 /* Preserve the version that was really wanted */
784 memcpy(&uc_fw->file_wanted, &file_ideal, sizeof(uc_fw->file_wanted));
785
786 gt_notice(gt, "%s firmware %s (%d.%d) is recommended, but only %s (%d.%d) was found\n",
787 intel_uc_fw_type_repr(uc_fw->type),
788 uc_fw->file_wanted.path,
789 uc_fw->file_wanted.ver.major, uc_fw->file_wanted.ver.minor,
790 uc_fw->file_selected.path,
791 uc_fw->file_selected.ver.major, uc_fw->file_selected.ver.minor);
792 gt_info(gt, "Consider updating your linux-firmware pkg or downloading from %s\n",
793 INTEL_UC_FIRMWARE_URL);
794 }
795
796 if (HAS_LMEM(i915)) {
797 obj = i915_gem_object_create_lmem_from_data(i915, fw->data, fw->size);
798 if (!IS_ERR(obj))
799 obj->flags |= I915_BO_ALLOC_PM_EARLY;
800 } else {
801 obj = i915_gem_object_create_shmem_from_data(i915, fw->data, fw->size);
802 }
803
804 if (IS_ERR(obj)) {
805 err = PTR_ERR(obj);
806 goto fail;
807 }
808
809 uc_fw->obj = obj;
810 uc_fw->size = fw->size;
811 intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_AVAILABLE);
812
813 release_firmware(fw);
814 return 0;
815
816 fail:
817 intel_uc_fw_change_status(uc_fw, err == -ENOENT ?
818 INTEL_UC_FIRMWARE_MISSING :
819 INTEL_UC_FIRMWARE_ERROR);
820
--> 821 gt_probe_error(gt, "%s firmware %s: fetch failed %pe\n",
822 intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path, ERR_PTR(err));
823 gt_info(gt, "%s firmware(s) can be downloaded from %s\n",
824 intel_uc_fw_type_repr(uc_fw->type), INTEL_UC_FIRMWARE_URL);
825
826 release_firmware(fw); /* OK even if fw is NULL */
827 return err;
828 }
regards,
dan carpenter
